Hi Pavel,
I made another patch, and it is uploaded to
http://cr.openjdk.java.net/~zhouyx/7089914/webrev.01/ .
The button.focus is now similar to a windows property, it will
be loaded when other windows properties are loaded; and it
stays in the same priority as other windows properties so user
defined value won't be overwritten.
And I also found a bug in WindowsRadioButtonUI which
blocks radiobutton and checkbox from reloading their styles,
its fix(override the uninstallDefaults ) is included in the webrev
as well.
A simple testcase which includes checkbox and radiobutton
is here:
/////////////////////////////
import java.awt.BorderLayout;
import java.awt.EventQueue;
import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JCheckBox;
import javax.swing.JFrame;
import javax.swing.JRadioButton;
import javax.swing.JToolBar;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;
public class TestButton {
private JFrame frame;
/**
* Launch the application.
*/
public static void main(String[] args) {
try {
UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
} catch (ClassNotFoundException | InstantiationException
| IllegalAccessException | UnsupportedLookAndFeelException
e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
EventQueue.invokeLater(new Runnable() {
public void run() {
try {
TestButton window = new TestButton();
window.frame.setVisible(true);
} catch (Exception e) {
e.printStackTrace();
}
}
});
}
/**
* Create the application.
*/
public TestButton() {
initialize();
}
/**
* Initialize the contents of the frame.
*/
private void initialize() {
frame = new JFrame();
frame.setBounds(100, 100, 450, 300);
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
JToolBar toolBar = new JToolBar();
frame.getContentPane().add(toolBar, BorderLayout.NORTH);
JButton btnNewButton = new JButton(new ImageIcon("testicon.png"));
toolBar.add(btnNewButton);
JButton btnNewButton_1 = new JButton("New button");
frame.getContentPane().add(btnNewButton_1, BorderLayout.CENTER);
JCheckBox chckbxNewCheckBox = new JCheckBox("New check box");
frame.getContentPane().add(chckbxNewCheckBox, BorderLayout.WEST);
JRadioButton rdbtnNewRadioButton = new JRadioButton("New radio
button");
frame.getContentPane().add(rdbtnNewRadioButton, BorderLayout.EAST);
}
}
////////////////////////////////
On Thu, Nov 24, 2011 at 2:23 AM, Pavel Porvatov
<pavel.porva...@oracle.com>wrote:
> Hi Sean,
>
> Hi Pavel,
>
> I rewrite a testcase, which is attached as TestButton.java . It has a
> toolbar and two buttons,
> one in toolbar and the other in contentpane. The one in toolbar use an
> image which is
> attached as testicon.png . I don't know how to write an automatic
> testcase for this one
> because it changes windows settings.
>
> Sometimes it's really hard to write automatic test. In your case SwingSet2
> demo and an appropriate instruction for testers is enough...
>
>
> Just run TestButton and change windows to high contrast mode and you
> can find the focus
> is not painted.
>
> I attached the patch in webrev.zip. It just checks if button
> background is black when
> windows properties changed. Please have a look.
>
> It seems your fix adds the following regression:
> somebody can install own "Button.focus" value, but your code puts own
> value over the users one
>
> Regards, Pavel
>
>
>
> On Sat, Oct 29, 2011 at 9:14 PM, Pavel Porvatov <
> pavel.porva...@oracle.com> wrote:
>
>> Hi Sean,
>>
>>
>>
>> It seems the black color for focus is set intentionally. If we set it
>> to "ControlTextColor ",
>> the focus color may become red in above testcase, that's not what we want.
>> And I changed the color for all items listed with "3D object", the
>> focus remains black;
>> maybe windows just uses "black" for focus color in normal mode, and
>> another color for
>> high contrast mode.
>>
>> However, the original patch posted is not right in this scenario.
>> I'll modify it. How about
>> just uses white for high contrast mode ? As it simply uses black for
>> normal mode.
>>
>> I'm not sure you can determine if high contrast mode is set... Every
>> heuristic function for selection color can fail in some situation.
>>
>> If somebody can take a look at source of
>> ControlPaint::DrawFocusRectangle(Graphics, Rectangle) method (see
>> http://msdn.microsoft.com/en-us/library/k2czzc46.aspx) and find out
>> which colors uses .NET....
>>
>> Regards, Pavel
>>
>>
>> On Wed, Oct 26, 2011 at 11:12 PM, Pavel Porvatov <
>> pavel.porva...@oracle.com> wrote:
>>
>>> Hi Sean,
>>>
>>> Hi Pavel,
>>>
>>> From your image, I agree the focus color is not always the same
>>> with ControlTextColor,
>>> but I cannot recreate it. When I changed color of "3D objects" to red,
>>> I got another image.
>>> Please have a look.
>>>
>>> It seems you changed Color1, but not Color (which a little bit lower
>>> then Color1)....
>>>
>>> I think your suggestion is reasonable, we'd better use the focus
>>> color from windows, but
>>> it maybe a problem to keep 100% the same, I still not found if there is
>>> a document for the
>>> focus color.
>>>
>>> Yes, the MS documentation about focus color is the best way to fix the
>>> bug. Can anybody point to such document?
>>>
>>> Regards, Pavel
>>>
>>>
>>> On Fri, Sep 16, 2011 at 7:06 PM, Pavel Porvatov <
>>> pavel.porva...@oracle.com> wrote:
>>>
>>>> Hi Neil,
>>>>
>>>> On Thu, 2011-09-15 at 17:04 +0400, Pavel Porvatov wrote:
>>>>>
>>>>>> Hi Neil,
>>>>>>
>>>>>>> On Wed, 2011-09-14 at 14:14 +0800, Sean Chou wrote:
>>>>>>>
>>>>>>>> Hi Pavel,
>>>>>>>>
>>>>>>>>
>>>>>>>> I reported a bug there yesterday,
>>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7089914
>>>>>>>> So far, I'm not sure if Windows use ControlTextColor, I'll check
>>>>>>>> it.
>>>>>>>>
>>>>>>>> For ease of review, I've uploaded Sean's change as a webrev [1].
>>>>>>>
>>>>>>> With the change, I see the following focus-related color settings in
>>>>>>> the
>>>>>>> WindowsLookAndFeel:
>>>>>>>
>>>>>>> Button.focus: ControlTextColor
>>>>>>> Checkbox.focus: ControlTextColor
>>>>>>> RadioButton.focus: ControlTextColor
>>>>>>> Slider.focus: ControlDarkShadowColor
>>>>>>> TabbedPane.focus: ControlTextColor
>>>>>>> ToggleButton.focus: ControlTextColor
>>>>>>>
>>>>>>> So the change of setting for Button, Checkbox and RadioButton
>>>>>>> conforms
>>>>>>> to what is already used for TabbedPane and ToggleButton.
>>>>>>>
>>>>>> But doesn't conform to Slider.focus...
>>>>>>
>>>>> Are you recommending that Slider.focus should be changed to
>>>>> ControlTextColor too ?
>>>>>
>>>> No, I meant that we cannot fix some bugs by copy-paste method.
>>>>
>>>>> From it's name, it's not entirely obvious to me that
>>>>>>>> 'ControlTextColor'
>>>>>>>>
>>>>>>> is really the ideal setting to use here, but it's also clear that
>>>>>>> it's a
>>>>>>> far better setting to use than the current hard-coded 'black'.
>>>>>>>
>>>>>> Yes, of course. The last question is which color is correct. We can't
>>>>>> change one incorrect color to another incorrect color...
>>>>>>
>>>>> I guess I hope that some knowledgeable person might be able to suggest
>>>>> /
>>>>> corroborate / refute the choice of setting here.
>>>>>
>>>>> It seems worse to consider sticking with a hard-coded, un-configurable
>>>>> value that has been demonstrated to cause problems, than to use a
>>>>> setting whose value can at least be configured, in practice fixes the
>>>>> problem's symptoms, and is already used in most other similar contexts
>>>>> within the same look& feel.
>>>>>
>>>>>
>>>>> Suggestions for how to improve things further are always welcome.
>>>>>
>>>> Your points sounds good. But as I said: we can't change one incorrect
>>>> color to another incorrect color (doesn't matter configurable it or not). I
>>>> attached the screenshot that shows that ControlTextColor is not always
>>>> equal to color of selection frame (to reproduce this image press the
>>>> Advanced button and change color of "3D objects" to red).
>>>>
>>>> Regards, Pavel
>>>>
>>>
>>>
>>>
>>> --
>>> Best Regards,
>>> Sean Chou
>>>
>>>
>>>
>>
>>
>> --
>> Best Regards,
>> Sean Chou
>>
>>
>>
>
>
> --
> Best Regards,
> Sean Chou
>
>
>
--
Best Regards,
Sean Chou
