RFR: 8015854: [macosx] JButton's HTML ImageView adding unwanted padding [v5]

[Alexey Ivanov]

On Fri, 25 Feb 2022 00:42:10 GMT, DamonGuy <duke at openjdk.java.net> wrote:

>> Html does not fit in JButton at certain sizes because default Insets cause html to be displayed off-center. 
>> 
>> Changes made to SwingUtilities.java layoutCompoundLabelImpl method to enable clipping if html does not fit, similar to regular text. AquaButtonUI.java now detects when html does not fit, and an implementation for alternate insets that are recursively tested for regular text inside layoutAndGetText() are now also being used for html content.
>> 
>> Created test (Bug8015854.java) with the same format as the test described on the issue. The button is of size 37x37 with an image of a red square sized 19x19. The test checks for red pixels on the edges of where the square image should be from the center of the button. The test fails with the non-changed jdk, but passes with the changes.
>
> DamonGuy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Changed robot autodelay time and added wait after creating gui. Added try finally block for frame disposal.

Changes requested by aivanov (Reviewer).

src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 313:

> 311:             // use zero insets for view since layout only handles text calculations
> 312:             text = layoutAndGetText(g, b, aquaBorder, new
> 313:                     Insets(0,0,0,0), viewRect, iconRect, textRect);

Please do not break object instantiation: always keep `new` and class name on the same line.

test/jdk/javax/swing/JButton/HtmlButtonImageTest/HtmlButtonImageTest.java line 67:

> 65:             } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
> 66:                 throw new RuntimeException("Unsupported LookAndFeel: " + e);
> 67:             }

You can let these exceptions propagate up and be thrown from `main` directly.

It was discussed in the comments that this bug is applicable to all Look-and-Feels. But the test is macOS- and Aqua-specific.

test/jdk/javax/swing/JButton/HtmlButtonImageTest/HtmlButtonImageTest.java line 101:

> 99: 
> 100:     private static void createAndShowGUI()
> 101:     {

The opening brace should on the same line as the method declaration.

test/jdk/javax/swing/JButton/HtmlButtonImageTest/HtmlButtonImageTest.java line 120:

> 118:     private static void setupCenterCoord() {
> 119:         // adjust coordinates to be the center of the button
> 120:         point = button.getLocationOnScreen();

`button` is a Swing component and must be accessed from EDT only.

test/jdk/javax/swing/JButton/HtmlButtonImageTest/HtmlButtonImageTest.java line 127:

> 125:     private static boolean checkRedness(Color c) {
> 126:         // checks for redness since anti-aliasing causes edges to be not exactly 255,0,0 rgb values
> 127:         if(c.getRed() > 250 && c.getBlue() < 10 && c.getGreen() < 10) {

The should be a space between `if` and the opening parenthesis.

test/jdk/javax/swing/JButton/HtmlButtonImageTest/HtmlButtonImageTest.java line 141:

> 139:             System.out.println("-- Passed");
> 140:         }
> 141:     }

I suggest using variable arguments:
Suggestion:

    private static void testImageCentering(Color... colors) {
        // check if all colors at each edge of square are red
        for (Color c : colors) {
            if (!checkRedness(c)) {
                throw new RuntimeException("HTML image not centered in button");
            }
        }
       System.out.println("-- Passed");
    }

The call to this function remains the same. And you can easily add additional points to be checked if needed.

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

PR: https://git.openjdk.java.net/jdk/pull/7310