8139173: [macosx] JInternalFrame shadow is not properly drawn #7228

toshiona · 2022-01-26T06:53:42Z

JInternalFrame's bottom area is not properly drawn with Aqua LAF.
This problem remained for long time, but we recognized it recently.

According to the bug report, it depends on MacOS's version. I don't have old ones (10.10 and 10.11 in the report), but the current MacOS can recreate the issue. I confirmed the following OS versions recreated this issue and this patch could solve it.

Mojave 10.14.6
Catalina 10.15.7
Big Sur 11.6.2
Monterey 12.1

jtreg "javax/swing" and "java/awt" have no regression.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8139173: [macosx] JInternalFrame shadow is not properly drawn

Reviewers

Phil Race (@prrace - Reviewer) ⚠️ Review applies to 44ea0e1
Sergey Bylokhov (@mrserb - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7228/head:pull/7228
$ git checkout pull/7228

Update a local copy of the PR:
$ git checkout pull/7228
$ git pull https://git.openjdk.java.net/jdk pull/7228/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7228

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7228.diff

bridgekeeper · 2022-01-26T06:54:51Z

👋 Welcome back tnakamura! 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 · 2022-01-26T06:57:08Z

@toshiona 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 · 2022-01-26T07:16:50Z

Webrevs

01: Full - Incremental (422a9c33)
00: Full (44ea0e12)

prrace

Looks so much better.

prrace · 2022-01-27T00:36:12Z

test/jdk/javax/swing/plaf/aqua/JInternalFrameBorderTest.java

+        try {
+            UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel");
+        } catch (Exception e) {
+            throw new RuntimeException("Cannot initialize Aqua L&F");


Really this should not be needed since it is the default. But it should at least stop folks running it on Windows.

openjdk · 2022-01-27T00:38:29Z

@toshiona 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:

8139173: [macosx] JInternalFrame shadow is not properly drawn

Reviewed-by: prr, serb

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 64 new commits pushed to the master branch:

0e70d45: 8280950: RandomGenerator:NextDouble() default behavior non conformant after JDK-8280550 fix
1ea0146: 8281007: Test jdk/javadoc/doclet/checkStylesheetClasses/CheckStylesheetClasses.java fails after JDK-8280738
9c0104b: 8221642: AccessibleObject::setAccessible throws NPE when invoked by JNI code with no java frame on stack
4dbebb6: 8280534: Enable compile-time doclint reference checking
4191b2b: 8275337: C1: assert(false) failed: live_in set of first block must be empty
96d0df7: 8272984: javadoc support for reproducible builds
ee3be0b: 8280488: doclint reference checks withstand warning suppression
74921e8: 8280738: Minor cleanup for HtmlStyle
3916561: 8280543: Update the "java" and "jcmd" tool specification for CDS
f991891: 8280949: Correct the references for the Java Security Standard Algorithm Names specification
... and 54 more: https://git.openjdk.java.net/jdk/compare/a183bfb436a7dd998e602c2d16486e88c390fca1...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@prrace, @mrserb) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mrserb · 2022-01-27T02:30:54Z

test/jdk/javax/swing/plaf/aqua/JInternalFrameBorderTest.java

+        robot.waitForIdle();
+        robot.delay(1000);
+
+        Point p = internalFrame.getLocationOnScreen();


The internalFrame is a Swing component so we should call all its methods on EDT.

mrserb · 2022-01-27T02:32:43Z

src/java.desktop/macosx/classes/com/apple/laf/AquaInternalFrameBorder.java

@@ -380,8 +380,6 @@ public void drawWindowTitle(final Graphics g, final JInternalFrame frame, final
        final int w = inW;
        int h = inH;

-        h = metrics.titleBarHeight + inH;


Just a suggestion, after the current change it is possible to align the h and other x/y/w vars and make it final.

toshiona · 2022-01-27T04:29:55Z

Thank you for reviewing, @prrace @mrserb.
Updated the patch. This testcase is limited to mac only.

@requires (os.family == "mac")

toshiona · 2022-02-01T04:37:36Z

/integrate

openjdk · 2022-02-01T04:38:47Z

@toshiona
Your change (at version 422a9c3) is now ready to be sponsored by a Committer.

toshiona · 2022-02-01T04:40:55Z

@prrace @mrserb
Thank you for the review. Could you sponsor this patch?

prrace · 2022-02-06T18:38:08Z

/sponsor

openjdk · 2022-02-06T18:39:56Z

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

77b0240: 8281183: RandomGenerator:NextDouble() default behavior partially fixed by JDK-8280950
42e272e: 8281289: Improve with List.copyOf
48523b0: 8281049: man page update for jstatd Security Manager dependency removal
8e4ef81: 8280767: -XX:ArchiveClassesAtExit does not archive BoundMethodHandle$Species classes
f5d6fdd: 8280476: [macOS] : hotspot arm64 bug exposed by latest clang
d4b99bc: 8281120: G1: Rename G1BlockOffsetTablePart::alloc_block to update_for_block
66b2c3b: 8280948: [TESTBUG] Write a regression test for JDK-4659800
7207f2a: Merge
01f93dd: 8279385: [test] Adjust sun/security/pkcs12/KeytoolOpensslInteropTest.java after 8278344
3d926dd: 8277795: ldap connection timeout not honoured under contention
... and 112 more: https://git.openjdk.java.net/jdk/compare/a183bfb436a7dd998e602c2d16486e88c390fca1...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-02-06T18:40:11Z

@prrace @toshiona Pushed as commit f7814c1.

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

8139173: [macosx] JInternalFrame shadow is not properly drawn

44ea0e1

openjdk bot added the client client-libs-dev@openjdk.org label Jan 26, 2022

toshiona marked this pull request as ready for review January 26, 2022 07:11

openjdk bot added the rfr Pull request is ready for review label Jan 26, 2022

prrace approved these changes Jan 27, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 27, 2022

mrserb reviewed Jan 27, 2022

View reviewed changes

Applied review comments

422a9c3

mrserb approved these changes Feb 1, 2022

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Feb 1, 2022

openjdk bot added the integrated Pull request has been integrated label Feb 6, 2022

openjdk bot closed this Feb 6, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8139173: [macosx] JInternalFrame shadow is not properly drawn #7228

8139173: [macosx] JInternalFrame shadow is not properly drawn #7228

toshiona commented Jan 26, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 26, 2022

openjdk bot commented Jan 26, 2022

mlbridge bot commented Jan 26, 2022 •

edited

Loading

prrace left a comment

prrace Jan 27, 2022

openjdk bot commented Jan 27, 2022 •

edited

Loading

mrserb Jan 27, 2022

mrserb Jan 27, 2022

toshiona commented Jan 27, 2022

toshiona commented Feb 1, 2022

openjdk bot commented Feb 1, 2022

toshiona commented Feb 1, 2022

prrace commented Feb 6, 2022

openjdk bot commented Feb 6, 2022

openjdk bot commented Feb 6, 2022

8139173: [macosx] JInternalFrame shadow is not properly drawn #7228

8139173: [macosx] JInternalFrame shadow is not properly drawn #7228

Conversation

toshiona commented Jan 26, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jan 26, 2022

openjdk bot commented Jan 26, 2022

mlbridge bot commented Jan 26, 2022 • edited Loading

Webrevs

prrace left a comment

Choose a reason for hiding this comment

prrace Jan 27, 2022

Choose a reason for hiding this comment

openjdk bot commented Jan 27, 2022 • edited Loading

mrserb Jan 27, 2022

Choose a reason for hiding this comment

mrserb Jan 27, 2022

Choose a reason for hiding this comment

toshiona commented Jan 27, 2022

toshiona commented Feb 1, 2022

openjdk bot commented Feb 1, 2022

toshiona commented Feb 1, 2022

prrace commented Feb 6, 2022

openjdk bot commented Feb 6, 2022

openjdk bot commented Feb 6, 2022

toshiona commented Jan 26, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 26, 2022 •

edited

Loading

openjdk bot commented Jan 27, 2022 •

edited

Loading