On Sat, 11 Feb 2023 00:23:05 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.
Changes requested by honkar (Committer).
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 78:
> 76: import com.apple.laf.AquaUtils.RecyclableSingleton;
> 77: import com.apple.laf.AquaUtils.RecyclableSingletonFromDefaultConstructor;
> 78: import sun.swing.SwingUtilities2;
Copyright year needs to be updated for AquaButtonUI.
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 && ((AbstractButton) c).getText().contains("<img
> ")) {
You might want to test a combination of HTML image + icon added to JButton, to
make sure the fix works correctly under different combinations.
Suggestion: Does having `((AbstractButton) c).getIcon() == null` make it more
robust than checking for html tag patterns ?
test/jdk/javax/swing/JButton/HtmlButtonWithTextAndIcon.java line 50:
> 48: import static java.awt.image.BufferedImage.TYPE_INT_ARGB;
> 49:
> 50: public class HtmlButtonWithTextAndIcon {
Would it be a good idea to combine `HtmlButtonWithTextAndIcon` &
`HtmlButtonImageTest` into one comprehensive test, since they are similar? This
would also make sure that all the cases related to JButton's HTML ImageView are
available at one place.
test/jdk/javax/swing/JButton/HtmlButtonWithTextAndIcon.java line 57:
> 55: private static final int BUTTON_WIDTH = 59;
> 56: private static final int BUTTON_HEIGHT = 28;
> 57: private static final int ICON_WIDTH = 16;
To make testing easier with different icon & button sizes, it is better to have
button width and height as a factor of icon size (1.5 times or greater) as
mentioned here [JDK-8015854](https://bugs.openjdk.org/browse/JDK-8015854).
-------------
PR: https://git.openjdk.org/jdk/pull/12520
