On Mon, 3 Mar 2025 11:29:33 GMT, Gopal Pattnaik <d...@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.
>
> Gopal Pattnaik has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Addressed Review comments
>  - Addressed Review comments

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

> 51: 
> 52:     private static CountDownLatch webviewLoadLatch = new 
> CountDownLatch(1);
> 53:     private Color colorBefore;

suggestion: make `colorBefore/After` _volatile_ since they are being accessed 
from different threads without any synchronization.
I think this should be sufficient (no need for `AtomicReference`).

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

> 87:         Util.runAndWait(() -> colorAfter = robot.getPixelColor(x, y));
> 88: 
> 89:         Assertions.assertNotEquals(colorBefore, colorAfter,

should we also test for non-null `colorBefore` ?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1719#discussion_r1979719322
PR Review Comment: https://git.openjdk.org/jfx/pull/1719#discussion_r1979721955

Reply via email to