On Fri, 10 May 2024 07:05:11 GMT, Abhishek Kumar <[email protected]> wrote:
> JTabbedPane's contentOpaque and tabsOpaque properties are not honored in Aqua > L&F. JTabbedPane's content area and tab background color are not as expected > when tabbedpane opacity is set to true or false. Fix is to handle the opacity > behavior correctly and inline with other LAF as well. > > Existing test `TestBackgroundScrollPolicy.java` failed with the proposed fix > and it is updated to run only for linux and windows platform because the > content area for tabbedpane is rendered to the width and height of tabbedpane > starting from (0, 0) position > (https://github.com/openjdk/jdk/blob/cf7c97732320d70de5f5725c920d5c3861a2c9c8/src/java.desktop/macosx/classes/com/apple/laf/AquaTabbedPaneUI.java#L684C16-L684C16) > and that leaves no place for tab area behind tabs. > > CI testing is green after this test update and link posted in JBS. src/java.desktop/macosx/classes/com/apple/laf/AquaLookAndFeel.java line 865: > 863: //"TabbedPane.darkShadow", table.get("controlDkShadow"), > 864: //"TabbedPane.focus", table.get("controlText"), > 865: "TabbedPane.opaque", false, Looks like `useOpaqueComponents` is normally set to `Boolean.TRUE`. But now we set it to `false`. Why not `Boolean.FALSE`? src/java.desktop/macosx/classes/com/apple/laf/AquaTabbedPaneUI.java line 58: > 56: protected Boolean isDefaultFocusReceiver = null; > 57: protected boolean hasAvoidedFirstFocus = false; > 58: private Color selectedColor; Suggestion: private Color selectedColor; test/jdk/javax/swing/JTabbedPane/TestBackgroundScrollPolicy.java line 51: > 49: for (UIManager.LookAndFeelInfo laf : > UIManager.getInstalledLookAndFeels()) { > 50: System.out.println("Testing L&F: " + laf.getClassName()); > 51: if (!laf.getClassName().contains("Aqua")) { Why can't this test be set to require windows or linux OS's in the test tags rather than use this if-statement? test/jdk/javax/swing/JTabbedPane/TestBackgroundScrollPolicy.java line 58: > 56: ROBOT.delay(1000); > 57: SwingUtilities.invokeAndWait(() -> test(laf)); > 58: ROBOT.delay(2000); Any reasons for these longer delays versus increasing the setAutoDelay and removing these? test/jdk/javax/swing/JTabbedPane/TestBackgroundScrollPolicy.java line 60: > 58: ROBOT.delay(2000); > 59: } finally { > 60: if (frame != null) SwingUtilities.invokeAndWait(() -> > frame.dispose()); I've seen other tests use this too but they still use the curly braces for the if-statement. I think it's better to keep the if-statement as a block, but the one line _does_ look neat. Suggestion: if (frame != null) { SwingUtilities.invokeAndWait(() -> frame.dispose()); } ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19170#discussion_r1600794138 PR Review Comment: https://git.openjdk.org/jdk/pull/19170#discussion_r1600784631 PR Review Comment: https://git.openjdk.org/jdk/pull/19170#discussion_r1600790803 PR Review Comment: https://git.openjdk.org/jdk/pull/19170#discussion_r1600786109 PR Review Comment: https://git.openjdk.org/jdk/pull/19170#discussion_r1600790258
