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