On Wed, 22 Feb 2023 00:02:10 GMT, Damon Nguyen <dngu...@openjdk.org> wrote:
>> Previous fix to HTML in AquaButtonUI fixed spacing issue for HTML images in
>> a JButton in Aqua L&F. The new issue comes from having text inside the HTML
>> as the button's text. If an icon is used, this icon exceeds the bounds of
>> the button and overlaps the border.
>>
>> Added additional logic to check if HTML contains an img. If so, apply the
>> previous fix. Otherwise, the original implementation for Aqua's JButton HTML
>> layout works correctly, so use this behavior.
>>
>> Added a test based on the provided test in the bug report. Automated the
>> test using a BufferedImage and changed the icon color to RED. Tested with
>> multiple runs on Aqua L&F, and the test passes with the update where it
>> fails without the update.
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove redundant check and reuse var.
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 343:
> 341: final String text;
> 342: final View v = (View)c.getClientProperty(BasicHTML.propertyKey);
> 343: if (v != null && b.getIcon() == null) {
I think you can store `b.getIcon` same as `text` variable and reuse it down
below too as it is now being called more than once
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 344:
> 342: final View v = (View)c.getClientProperty(BasicHTML.propertyKey);
> 343: if (v != null && b.getIcon() == null) {
> 344: // use zero insets for view when HTML contains an image
I guess it should be "**does not** contain an image", right?
test/jdk/javax/swing/JButton/HtmlButtonWithTextAndIcon.java line 27:
> 25: /* @test
> 26: * @bug 8302173
> 27: * @requires (os.family == "mac")
Is this required? HtmlButtonImageTest is being tested for all platforms even
though fix was in mac so I think we should make this test also
platform-agnostic..
-------------
PR: https://git.openjdk.org/jdk/pull/12520
