Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked #5082

Closed
wants to merge 7 commits into from
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -326,6 +326,20 @@ public void paint(final Graphics g, final JComponent c) {
}
}

protected void paintFocus(Graphics g, AbstractButton b,
Rectangle viewRect, Rectangle textRect, Rectangle iconRect) {
if (g instanceof Graphics2D) {
((Graphics2D)g).setStroke(new BasicStroke(3));
((Graphics2D)g).setRenderingHint(
RenderingHints.KEY_ANTIALIASING,
RenderingHints.VALUE_ANTIALIAS_ON);

}
Color ringColor = UIManager.getColor("Focus.color");
g.setColor(ringColor);
g.drawRoundRect(5, 3, b.getWidth() - 10, b.getHeight() - 7, 15, 15);
Copy link
Member

@mrserb mrserb Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to restore the properties of Graphics2D?

Copy link
Member Author

@azuev-java azuev-java Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to restore the properties of Graphics2D?

I might as well.

Copy link
Member

@mrserb mrserb Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the color property?

Copy link
Member Author

@azuev-java azuev-java Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the color property?

That would be unnecessary, whoever does painting does not just assumes that the color is set to his preferred choice, everywhere in the code color is set up before doing any painting and i do not see anywhere that old color being preserved.

Copy link
Member

@mrserb mrserb Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can trace the usage of graphics.getColor() for example in AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color property is usually saved and then restored.

Copy link
Member Author

@azuev-java azuev-java Aug 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can trace the usage of graphics.getColor() for example in AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color property is usually saved and then restored.

A few lines above paintFocus() is called there is a section that sets color to something different in case of opaque button and that does not affect the next step - text painting - since text painter will grab text color and set the graphics draw color to it. However in the interest of moving forward i will ad saving the color - after al it is not a performance critical task.

Copy link
Contributor

@prsadhuk prsadhuk Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked if button does not have any text? It might affect the drawRoundRect values which are hardcoded now...In MetalButtonUI#paintFocus, it seems they cater to Button focus ring with and without text by taking care of setBounds(). Do we need to do something similar here?

Copy link
Member Author

@azuev-java azuev-java Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked if button does not have any text?

Yes, i did. Neither my code nor native focus painter is affected by it.

}

protected String layoutAndGetText(final Graphics g, final AbstractButton b, final AquaButtonBorder aquaBorder, final Insets i, Rectangle viewRect, Rectangle iconRect, Rectangle textRect) {
// re-initialize the view rect to the selected insets
viewRect.x = i.left;
@@ -0,0 +1,91 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @requires (os.family == "mac")
* @bug 8269951
* @summary Test checks that focus is painted on JButton even
* when borders turned off
* @library ../../regtesthelpers
* @build Util
* @run main AquaButtonFocusTest
*/


import javax.swing.JButton;
import javax.swing.UIManager;
import java.awt.Graphics;
import java.awt.image.BufferedImage;

public class AquaButtonFocusTest {
public static void main(String[] args) {
new AquaButtonFocusTest().performTest();
}

public void performTest() {
try {
UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel");
} catch (Exception e) {
throw new RuntimeException("Can not initialize Aqua L&F");
}

FocusableButton one = new FocusableButton("One");
one.setSize(100, 100);
one.setBorderPainted(false);
BufferedImage noFocus = new BufferedImage(200, 100, BufferedImage.TYPE_INT_ARGB);
Graphics g = noFocus.createGraphics();
one.paint(g);
g.dispose();
BufferedImage focus = new BufferedImage(200, 100, BufferedImage.TYPE_INT_ARGB);
one.setFocusOwner(true);
g = focus.createGraphics();
one.paint(g);
g.dispose();
if (Util.compareBufferedImages(noFocus, focus)) {
throw new RuntimeException("Focus is not painted on JButton");
}
}

class FocusableButton extends JButton {
private boolean focusOwner = false;

public FocusableButton(String label) {
super(label);
}

public void setFocusOwner(boolean focused) {
this.focusOwner = focused;
}

@Override
public boolean isFocusOwner() {
return focusOwner;
}

@Override
public boolean hasFocus() {
return this.focusOwner;
}
}
}