RFR: 8331619: TabbedPane's contentOpaque, tabsOpaque and setOpaque doesn't work properly in Aqua LAF

[Damon Nguyen]

On Fri, 10 May 2024 07:05:11 GMT, Abhishek Kumar <abhiscxk at openjdk.org> 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