RFR: 8302173: Button border overlaps with button icon on macOS system LaF [v4]
Prasanta Sadhukhan
psadhukhan at openjdk.org
Wed Feb 22 06:38:45 UTC 2023
On Wed, 22 Feb 2023 00:02:10 GMT, Damon Nguyen <dnguyen at 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
More information about the client-libs-dev
mailing list