RFR: 8327478: Add System test to verify TextSelection issue for webkit-617.1

[Kevin Rushforth]

On Fri, 21 Feb 2025 06:59:52 GMT, Gopal Pattnaik <duke at openjdk.org> wrote:

> There was no test included with the fix for [JDK-8326989](https://bugs.openjdk.org/browse/JDK-8326989),
> 
> Hence we are adding a system test now.
> 
> Test is written as 
> 1. Load html content in web view.
> 2. pick the color of mouse pointer.
> 3. Perform double click.
> 4. pick the color again.
>> expected bahaviour: colour picked in step 2 and 4 should not match.
> 
> Verification:
> The test passes with latest webkit source
> Also verified that test fails when the fix for [JDK-8326989](https://bugs.openjdk.org/browse/JDK-8326989) is reverted.

@Gopalora Please enable GHA tests on your repo.

I'll provide a few comments now, and formally review it once your OCA status is verified.

The latest changes look good. I'll formally review it once your OCA status is verified.

tests/system/src/test/java/test/robot/javafx/web/TextSelectionTest.java line 101:

> 99: 
> 100:         Util.runAndWait(() -> {
> 101:             robot.mouseMove(1, 1);

Minor: use the utility method `Util.parkCursor(robot);` instead

tests/system/src/test/java/test/robot/javafx/web/TextSelectionTest.java line 110:

> 108:             Util.sleep(50);
> 109:             robot.mousePress(MouseButton.PRIMARY);
> 110:             robot.mouseRelease(MouseButton.PRIMARY);

Minor: You can call `robot.mouseClick(MouseButton)` instead of separate calls to press/release.

Not so minor: You shouldn't' sleep in the FX app thread. Instead, break this block into two separate lambdas passed into to separate calls to `runAndWait` with the sleep in between them running in the test thread. Like this:


        Util.runAndWait(() -> {
            ...
            robot.mouseClick(MouseButton.PRIMARY);
        });
        Util.sleep(50);
        Util.runAndWait(() -> {
            robot.mouseClick(MouseButton.PRIMARY);
        });


Suggestion: consider creating a `doubleClick` utility method in the test `Util` class.

tests/system/src/test/java/test/robot/javafx/web/TextSelectionTest.java line 116:

> 114: 
> 115:         Util.runAndWait(() -> {
> 116:             robot.mouseMove(1, 1);

Minor: use the utility method `Util.parkCursor(robot);` instead

tests/system/src/test/java/test/util/Util.java line 340:

> 338:      * Makes double click of the mouse left button.
> 339:      */
> 340:     public static void doubleClick(Robot robot, int x, int y) {

Since this is now a general-purpose utility, I'd prefer to separate out the mouse move from the double-click (with the mouse move being done in the test itself) and just have this be `void doubleClick(Robot robot)`.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1719#issuecomment-2682052978
PR Comment: https://git.openjdk.org/jfx/pull/1719#issuecomment-2690829082
PR Review Comment: https://git.openjdk.org/jfx/pull/1719#discussion_r1965697976
PR Review Comment: https://git.openjdk.org/jfx/pull/1719#discussion_r1965712264
PR Review Comment: https://git.openjdk.org/jfx/pull/1719#discussion_r1965699563
PR Review Comment: https://git.openjdk.org/jfx/pull/1719#discussion_r1971745158