On Thu, 12 Jun 2025 23:33:46 GMT, Damon Nguyen <dngu...@openjdk.org> wrote:

>> This change is to restore the original intent of the test by updating the 
>> instructions to check that the type of Cursor is preserved when clicked and 
>> dragged. Now the test correctly has instructions to check that an I-beam 
>> cursor stays an I-beam until released over a List with its cursor being 
>> updated to a Hand cursor.
>> 
>> There is a bug where this does not correctly update in macOS (found in 
>> [JDK-7177297](https://bugs.openjdk.org/browse/JDK-7177297)). So, this test 
>> needs to be problem-listed.
>> 
>> I have confirmed that preserving the cursor image when dragging is native 
>> behavior across macOS, Windows, and Ubuntu. And I have checked that the test 
>> passes on both Windows and Ubuntu, while macOS fails and immediately updates 
>> the cursor as it leaves the TextArea.
>
> Damon Nguyen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update test to avoid PFJ

The custom manual test frame changes LGTM apart for minor inline suggestions 
and works as expected on windows.

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 31:

> 29:  */
> 30: 
> 31: import java.awt.Button;

Copyright year needs to be updated

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 80:

> 78:         frame.add(panel);
> 79:         frame.setSize(300, 150);
> 80:         frame.setLocation(450, 400);

Allows more space between the windows

Suggestion:

        frame.setLocation(450, 500);

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 85:

> 83: 
> 84:     static void createInstructionsFrame() {
> 85:         String INSTRUCTIONS = """

Since it is local var (non-static & non-final var)

Suggestion:

        String instructions = """

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 124:

> 122:         instructionsFrame.pack();
> 123:         instructionsFrame.setLocation(300, 100);
> 124:         instructionsFrame.setVisible(true);

Rearranging the lines provides better clarity. mainPanel layout can be set to 
BorderLayout for better placement of the instruction panel and button panel.

Suggestion:

        instructionsFrame = new Frame("Test Instructions");
        Panel mainPanel = new Panel(new BorderLayout());
        TextArea textArea = new TextArea(instructions, 15, 60,
                                         TextArea.SCROLLBARS_NONE);

        Panel btnPanel = new Panel();
        Button passBtn = new Button("PASS");
        Button failBtn = new Button("FAIL");
        btnPanel.add(passBtn);
        btnPanel.add(failBtn);

        passBtn.addActionListener(e -> disposeFrames());
        failBtn.addActionListener(e -> {
            disposeFrames();
            throw new RuntimeException("Test Failed");
        });

        mainPanel.add(textArea, BorderLayout.CENTER);
        mainPanel.add(btnPanel, BorderLayout.SOUTH);

        instructionsFrame.add(mainPanel);
        instructionsFrame.pack();
        instructionsFrame.setLocation(300, 100);
        instructionsFrame.setVisible(true);

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

Marked as reviewed by honkar (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25705#pullrequestreview-2923069668
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2143974229
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2143967233
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2143975147
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2143964455

Reply via email to