8343170: java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java does not show the default cursor #22055

DamonGuy · 2024-11-13T01:14:26Z

Change test instructions and drawString to say "Text" instead of "Default" for cursor type. Added instructions to test setCursor for the frame as well. Also increased the column count to fit the instructions in linux, as it was mentioned that the instructions didn't fit there.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8343170: java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java does not show the default cursor (Bug - P4)

Reviewers

Alisen Chung (@alisenchung - Committer) Review applies to 77a7ccfb
Harshitha Onkar (@honkar-jdk - Reviewer)
Alexander Zvegintsev (@azvegint - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22055

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22055.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-11-13T01:14:55Z

👋 Welcome back dnguyen! 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 · 2024-11-13T01:15:12Z

@DamonGuy This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8343170: java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java does not show the default cursor

Reviewed-by: honkar, azvegint, achung

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1023 new commits pushed to the master branch:

a6277bb: 8340212: -Xshare:off -XX:CompressedClassSpaceBaseAddress=0x40001000000 crashes on macos-aarch64
165fd84: 8342987: Update --release 24 symbol information for JDK 24 build 27
06c44dd: 8345465: Fix performance regression on x64 after JDK-8345120
2979806: 8345795: Update copyright year to 2024 for hotspot in files where it was missed
7e73c43: 8345217: Parallel: Refactor PSParallelCompact::next_src_region
7d3a404: 8339019: Obsolete the UseLinuxPosixThreadCPUClocks flag
a606836: 8345744: Use C++ LINK_TYPE with SetupBuildLauncher in StaticLibs.gmk
fe0173d: 8345803: Update copyright year to 2024 for security in files where it was missed
2207749: 8344629: SSLSocketNoServerHelloClientShutdown test timeout
0b9e749: 8345390: [ubsan] systemDictionaryShared.cpp:964: member call on null pointer
... and 1013 more: https://git.openjdk.org/jdk/compare/7276a1bec0d90f63e9e433fdcdfd6564b70dc9bb...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2024-11-13T01:15:49Z

@DamonGuy The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-11-13T01:19:08Z

Webrevs

alisenchung · 2024-11-13T21:19:49Z

I notice one more cursor type (resize cursor?) in between the "move" panel and the panel containing the crosshair button and hand panel. I think it's caused by the JSplitPane resizing option. Since this test is about checking different cursor types while hovering parts of the frame, it might be worth either adding this cursor to the instructions or setting the JSplitPane to not be resizable to remove the cursor.

alisenchung

add frame title, probably add final to instructions field since you have it in all caps

DamonGuy · 2024-11-13T21:56:16Z

I notice one more cursor type (resize cursor?) in between the "move" panel and the panel containing the crosshair button and hand panel. I think it's caused by the JSplitPane resizing option. Since this test is about checking different cursor types while hovering parts of the frame, it might be worth either adding this cursor to the instructions or setting the JSplitPane to not be resizable to remove the cursor.

I saw this when initially converting the test, but since the original bug description and the test summary is checking for the functionality of setCursor(), I don't think it's worth adding a check for the resize cursor.

But I realized that the frame's wait cursor works on all OS's and the frame isn't part of the original test instructions anyway, so I repurposed the wait cursor for the pane instead. Simplifies the instructions too.

alisenchung · 2024-11-15T13:50:59Z

the frame isn't part of the original test instructions anyway

The original reproducer didn't have test instructions so it's more likely the person who wrote the instructions just forgot about testing the wait cursor. The idea of the original test is that there was a bug where setCursor works for JComponent and JFrame but doesn't work for JPanel.

So the left side of this test (move cursor) tests for JComponent,
the bottom right (hand) tests for JPanel,
the button (crosshair) tests for "various swing components",
and if you move the cursor outside all of these components onto the JFrame itself, it would test for the JFrame (wait cursor).

The test instructions probably missed the JFrame section of the test since the area on the test where the JFrame is tested is so small, but I think it is worth keeping it. To make the test area bigger you can set the JSplitPane to be added to the "North" of the frame instead of the "Center" and explicitly setSize(300,200) instead of pack(). If you choose to do this you should also add a step in the instructions to test the JFrame setCursor, something like
4. Verify that the cursor changes to the wait cursor when outside all other components at the bottom of the JFrame

alisenchung · 2024-11-15T13:58:25Z

If you want to simplify the test instructions between platforms I would rather change the move cursor to the text cursor and leave all the other cursors.

alisenchung

fix spacing error at line 107

alisenchung

looks good once whitespaces removed

test/jdk/java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java

honkar-jdk

LGTM

honkar-jdk · 2024-12-10T01:15:36Z

@DamonGuy Tested on windows and it works as expected, does the test instructions hold good on all platforms ?

DamonGuy · 2024-12-10T22:55:38Z

@DamonGuy Tested on windows and it works as expected, does the test instructions hold good on all platforms ?

Yup! Changed the cursor types so they're applicable on all platforms. Thanks

DamonGuy · 2024-12-10T23:51:34Z

/integrate

openjdk · 2024-12-10T23:52:23Z

Going to push as commit c631719.
Since your change was applied there have been 1032 commits pushed to the master branch:

29d648c: 8341781: Improve Min/Max node identities
4c39e9f: 8344924: Default CA certificates loaded despite request to use custom keystore
0f03554: 8342469: Improve API documentation for java.lang.classfile.instruction
9bd70ec: 8345888: Broken links in the JDK 24 JavaDoc API documentation, build 27
1def2d8: 8345700: tier{1,2,3}_compiler don't cover all compiler tests
d6b5264: 8345746: Remove :resourcehogs/compiler from :hotspot_slow_compiler
4f855d1: 8345818: Fix SM cleanup of parsing of System property resource.bundle.debug
a24b08f: 8345669: RISC-V: fix client build failure due to AlignVector after JDK-8343827
25d9deb: 8335367: [s390] Add support for load immediate on condition instructions.
a6277bb: 8340212: -Xshare:off -XX:CompressedClassSpaceBaseAddress=0x40001000000 crashes on macos-aarch64
... and 1022 more: https://git.openjdk.org/jdk/compare/7276a1bec0d90f63e9e433fdcdfd6564b70dc9bb...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-12-10T23:52:33Z

@DamonGuy Pushed as commit c631719.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Initial commit

1cea999

openjdk bot added the rfr Pull request is ready for review label Nov 13, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Nov 13, 2024

alisenchung reviewed Nov 13, 2024

View reviewed changes

Review comments

a72550e

alisenchung reviewed Nov 15, 2024

View reviewed changes

Review comment again

77a7ccf

openjdk bot removed the rfr Pull request is ready for review label Nov 15, 2024

DamonGuy requested a review from alisenchung November 15, 2024 17:42

alisenchung approved these changes Nov 19, 2024

View reviewed changes

test/jdk/java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java Outdated Show resolved Hide resolved

Fix whitespace

4377869

openjdk bot added the rfr Pull request is ready for review label Nov 20, 2024

honkar-jdk approved these changes Dec 10, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 10, 2024

azvegint approved these changes Dec 10, 2024

View reviewed changes

honkar-jdk approved these changes Dec 10, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Dec 10, 2024

openjdk bot closed this Dec 10, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8343170: java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java does not show the default cursor #22055

8343170: java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java does not show the default cursor #22055

DamonGuy commented Nov 13, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Nov 13, 2024

openjdk bot commented Nov 13, 2024 •

edited

Loading

openjdk bot commented Nov 13, 2024

mlbridge bot commented Nov 13, 2024 •

edited

Loading

alisenchung commented Nov 13, 2024

alisenchung left a comment

DamonGuy commented Nov 13, 2024

alisenchung commented Nov 15, 2024

alisenchung commented Nov 15, 2024

alisenchung left a comment

alisenchung left a comment

honkar-jdk left a comment

honkar-jdk commented Dec 10, 2024 •

edited

Loading

DamonGuy commented Dec 10, 2024

DamonGuy commented Dec 10, 2024

openjdk bot commented Dec 10, 2024

openjdk bot commented Dec 10, 2024

8343170: java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java does not show the default cursor #22055

8343170: java/awt/Cursor/JPanelCursorTest/JPanelCursorTest.java does not show the default cursor #22055

Conversation

DamonGuy commented Nov 13, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Nov 13, 2024

openjdk bot commented Nov 13, 2024 • edited Loading

openjdk bot commented Nov 13, 2024

mlbridge bot commented Nov 13, 2024 • edited Loading

Webrevs

alisenchung commented Nov 13, 2024

alisenchung left a comment

Choose a reason for hiding this comment

DamonGuy commented Nov 13, 2024

alisenchung commented Nov 15, 2024

alisenchung commented Nov 15, 2024

alisenchung left a comment

Choose a reason for hiding this comment

alisenchung left a comment

Choose a reason for hiding this comment

honkar-jdk left a comment

Choose a reason for hiding this comment

honkar-jdk commented Dec 10, 2024 • edited Loading

DamonGuy commented Dec 10, 2024

DamonGuy commented Dec 10, 2024

openjdk bot commented Dec 10, 2024

openjdk bot commented Dec 10, 2024

DamonGuy commented Nov 13, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Nov 13, 2024 •

edited

Loading

mlbridge bot commented Nov 13, 2024 •

edited

Loading

honkar-jdk commented Dec 10, 2024 •

edited

Loading