8365635: Add MOUSE_DRAG_DONE event type #1873

nlisker · 2025-08-16T20:44:28Z

Adds MOUSE_DRAG_DONE event type to MouseDragEvent and appropriate handlers.

/reviewers 2
/csr

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires a CSR request matching fixVersion jfx26 to be approved (needs to be created)
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8365635: Add MOUSE_DRAG_DONE event type (Enhancement - P3)

Reviewers

John Hendrikx (@hjohn - Reviewer) 🔄 Re-review required (review applies to 2fe9030b)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1873/head:pull/1873
$ git checkout pull/1873

Update a local copy of the PR:
$ git checkout pull/1873
$ git pull https://git.openjdk.org/jfx.git pull/1873/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1873

View PR using the GUI difftool:
$ git pr show -t 1873

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1873.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-08-16T20:45:09Z

👋 Welcome back nlisker! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-08-16T20:45:41Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-08-16T20:46:07Z

@nlisker
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

openjdk · 2025-08-16T20:46:40Z

@nlisker has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@nlisker please create a CSR request for issue JDK-8365635 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

nlisker · 2025-08-18T16:15:31Z

I noticed that the @see documentation tag doesn't work on properties. I'm not sure if it's a bug in javadoc.

mlbridge · 2025-08-18T16:20:06Z

Webrevs

01: Full - Incremental (cc11d675)
00: Full (2fe9030b)

hjohn

LGTM

nlisker · 2025-08-18T18:30:47Z

Forgot to attach a small manual test application I used for testing full DPR and DnD. Might be helpful for reviewers.

package main;

import javafx.application.Application;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.scene.input.ClipboardContent;
import javafx.scene.input.Dragboard;
import javafx.scene.input.MouseEvent;
import javafx.scene.input.TransferMode;
import javafx.scene.layout.HBox;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.scene.shape.Rectangle;
import javafx.stage.Stage;

public class MouseTest extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(@SuppressWarnings("exports") Stage stage) throws Exception {
        var sourceMouse = createSourceMouse();
        var sourceDrag = createSourceDrag();
        var target = createTarget();

        var scene = createScene(sourceMouse, sourceDrag, target);
        stage.setScene(scene);
        stage.show();
    }

    private Rectangle createSourceMouse() {
        var sourceMouse = new Rectangle(50, 50);
        sourceMouse.setId("sourceMouse");
        sourceMouse.setFill(Color.RED);
        sourceMouse.setOnDragDetected(e -> {
            sourceMouse.startFullDrag();
            System.out.println(e.getEventType() + " " + sourceMouse.getId());
            e.consume();
        });
        addDragListeners(sourceMouse);
        return sourceMouse;
    }

    private Rectangle createSourceDrag() {
        var sourceDrag = new Rectangle(50, 50);
        sourceDrag.setId("sourceDrag");
        sourceDrag.setFill(Color.GREEN);
        sourceDrag.setOnDragDetected(e -> {
            Dragboard db = sourceDrag.startDragAndDrop(TransferMode.ANY);
            var content = new ClipboardContent();
            content.putString("");
            db.setContent(content);
            System.out.println(e.getEventType() + " " + sourceDrag.getId());
            e.consume();
        });
        addDragListeners(sourceDrag);
        return sourceDrag;
    }

    private Rectangle createTarget() {
        var target = new Rectangle(100, 50);
        target.setId("target");
        target.setFill(Color.BLUE);
        target.setOnDragDetected(MouseEvent::consume);
        addDragListeners(target);
        return target;
    }

    private Scene createScene(Rectangle sourceMouse, Rectangle sourceDrag, Rectangle target) {
        var scene = new Scene(new VBox(10, new HBox(sourceMouse, sourceDrag), target), 300, 200);
        scene.setOnDragDetected(e -> {
            scene.startFullDrag();
            System.out.println(e.getEventType() + " scene");
        });

        scene.setOnDragDone(e -> {
            e.acceptTransferModes(TransferMode.ANY);
            System.out.println(e.getEventType() + " scene");
        });
        scene.setOnMouseDragReleased(e -> System.out.println(e.getEventType() + " scene"));
        scene.setOnMouseDragDone(e -> System.out.println(e.getEventType() + " scene"));
        return scene;
    }

    private static void addDragListeners(Node node) {
        node.setOnDragOver(e -> {
            e.acceptTransferModes(TransferMode.ANY);
            System.out.println(e.getEventType() + " " + node.getId());
        });
        node.setOnMouseDragOver(e -> System.out.println(e.getEventType() + " " + node.getId()));

        node.setOnDragDropped(e -> {
            e.acceptTransferModes(TransferMode.ANY);
            e.setDropCompleted(true);
            System.out.println(e.getEventType() + " " + node.getId());
        });
        node.setOnMouseDragReleased(e -> System.out.println(e.getEventType() + " " + node.getId()));

        node.setOnDragDone(e -> {
            e.acceptTransferModes(TransferMode.ANY);
            System.out.println(e.getEventType() + " " + node.getId());
        });
        node.setOnMouseDragDone(e -> System.out.println(e.getEventType() + " " + node.getId()));
    }
}

mstr2 · 2025-08-18T18:39:07Z

modules/javafx.graphics/src/main/java/javafx/scene/Node.java

+        return (eventHandlerProperties == null) ? null : eventHandlerProperties.getOnMouseDragDone();
+    }
+
+    /// Defines a function to be called when a full press-drag-release gesture ends with this node as its source.


This is the first instance of markdown-style javadoc comments in JavaFX. Do we need a discussion surrounding their use?

They were added in JDK 23, which we can use. I don't know if there's anything special to discuss. The JEP gives standard usage examples, which I've followed, so there's no "feature abuse" here either.

Well, they look very different from the other style. Usually matters of style are settled once in a style guide, which we don't have. But I think we should at least consciously agree that we're fine with having two different styles of documentation in the same project. Personally, I value uniformity of style more than the (little) added value that markdown comments bring.

+1 @mstr2 ..
The markdown style is quite different than traditional HTML base style. With increased use of markdown, the comments will look very different. We should keep the uniformity, unless we want to use any feature from markdown style that is not available in traditional style.

I think markdown style is ok, since it results in a more readable source.

The support in other tools might be lacking initially (for example, Eclipse does not refresh its javadoc pane with markdown as it does with the usual style)

Eclipse does not refresh its javadoc pane with markdown as it does with the usual style

Was fixed months ago in eclipse-jdt/eclipse.jdt.ui#2051. Not sure if a stable version was released yet.

andy-goryachev-oracle

I must be doing something wrong - I don't see MouseDragEvent.MOUSE_DRAG_DONE, or in fact MouseDragEvent.ANY sent to the listeners, with this version of the monkey tester:

https://github.com/andy-goryachev-oracle/MonkeyTest/tree/mouse.drag.done

The test page in question is
https://github.com/andy-goryachev-oracle/MonkeyTest/blob/mouse.drag.done/src/com/oracle/tools/fx/monkey/pages/DnDPage.java

andy-goryachev-oracle · 2025-08-29T22:19:04Z

modules/javafx.graphics/src/main/java/javafx/scene/Node.java

+        return (eventHandlerProperties == null) ? null : eventHandlerProperties.getOnMouseDragDone();
+    }
+
+    /// Defines a function to be called when a full press-drag-release gesture ends with this node as its source.


I think markdown style is ok, since it results in a more readable source.

The support in other tools might be lacking initially (for example, Eclipse does not refresh its javadoc pane with markdown as it does with the usual style)

modules/javafx.graphics/src/main/java/javafx/scene/input/MouseDragEvent.java

nlisker · 2025-08-29T23:15:11Z

I must be doing something wrong - I don't see MouseDragEvent.MOUSE_DRAG_DONE, or in fact MouseDragEvent.ANY sent to the listeners, with this version of the monkey tester:

https://github.com/andy-goryachev-oracle/MonkeyTest/tree/mouse.drag.done

The test page in question is https://github.com/andy-goryachev-oracle/MonkeyTest/blob/mouse.drag.done/src/com/oracle/tools/fx/monkey/pages/DnDPage.java

Did you try the sample program I posted?

andy-goryachev-oracle · 2025-08-29T23:17:07Z

Did you try the sample program

no, intentionally.

nlisker · 2025-08-29T23:17:26Z

In https://github.com/andy-goryachev-oracle/MonkeyTest/blob/mouse.drag.done/src/com/oracle/tools/fx/monkey/pages/DnDPage.java, I don't see you starting a press-drag-release process, only a DnD one.

andy-goryachev-oracle · 2025-09-02T17:02:23Z

Yes, calling startFullDrag() did the trick:

{event=MOUSE_DRAGGED, x/y=(-785, -106), screen=(19, 57), scene=-360, -67)}
{event=MOUSE-DRAG_DONE, x/y=(-785, -106), screen=(19, 57), scene=-360, -67)}
{event=MOUSE_RELEASED, x/y=(-785, -106), screen=(19, 57), scene=-360, -67)}

Question: should we explain in greater detail that the new event is coming during full drag only?

andy-goryachev-oracle · 2025-09-02T17:08:27Z

This is only tangentially related, but why in the drag and drop mode the coordinates for DRAG_DONE event are completely bogus when drop happens over a non-accepting target?

{event=DRAG_EXITED, x/y=(52, 32), screen=(1022, -1340), scene=(477, 71)}
{event=DRAG_EXITED_TARGET, x/y=(52, 32), screen=(1022, -1340), scene=(477, 71)}
{event=DRAG_DONE, x/y=(0, 0), screen=(0, 0), scene=(0, 0)}

nlisker · 2025-09-02T21:21:07Z

Question: should we explain in greater detail that the new event is coming during full drag only?

It's an event type of MouseDragEvent, which is the event used in full PDR, so I don't think there's an ambiguity here. I have a future docs change in the works around mouse events that clears up a lot of things.

nlisker · 2025-09-02T21:38:05Z

This is only tangentially related, but why in the drag and drop mode the coordinates for DRAG_DONE event are completely bogus when drop happens over a non-accepting target?

I don't know why it was done this way, but the coordinates are explicitly set to 0 from GlassScene.createDragboard and GlassSceneDnDEventHandler.handleDragEnd.

Initial commit

2fe9030

openjdk bot added the csr Need approved CSR to integrate pull request label Aug 16, 2025

nlisker marked this pull request as ready for review August 18, 2025 16:13

openjdk bot added the rfr Ready for review label Aug 18, 2025

hjohn approved these changes Aug 18, 2025

View reviewed changes

mstr2 reviewed Aug 18, 2025

View reviewed changes

kevinrushforth self-requested a review August 18, 2025 20:26

andy-goryachev-oracle reviewed Aug 29, 2025

View reviewed changes

Missing @SInCE

cc11d67

8365635: Add MOUSE_DRAG_DONE event type #1873

Are you sure you want to change the base?

8365635: Add MOUSE_DRAG_DONE event type #1873

Uh oh!

Conversation

nlisker commented Aug 16, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Aug 16, 2025

Uh oh!

openjdk bot commented Aug 16, 2025

Uh oh!

openjdk bot commented Aug 16, 2025

Uh oh!

openjdk bot commented Aug 16, 2025

Uh oh!

nlisker commented Aug 18, 2025

Uh oh!

mlbridge bot commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

hjohn left a comment

Choose a reason for hiding this comment

Uh oh!

nlisker commented Aug 18, 2025

Uh oh!

mstr2 Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

nlisker Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

mstr2 Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

arapte Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

nlisker Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nlisker commented Aug 29, 2025

Uh oh!

andy-goryachev-oracle commented Aug 29, 2025

Uh oh!

nlisker commented Aug 29, 2025

Uh oh!

andy-goryachev-oracle commented Sep 2, 2025

Uh oh!

andy-goryachev-oracle commented Sep 2, 2025

Uh oh!

nlisker commented Sep 2, 2025

Uh oh!

nlisker commented Sep 2, 2025

Uh oh!

Uh oh!

nlisker commented Aug 16, 2025 •

edited by openjdk bot

Loading

mlbridge bot commented Aug 18, 2025 •

edited

Loading