<Swing Dev> Request review for 7089914: Focus on image icons are not visible in javaws cache with high contrast mode

[Sean Chou]

Hi Pavel,

     Thanks for your suggestion, I didn't know DesktopProperty could be
used this way.
I modified it a little, it's
http://cr.openjdk.java.net/~zhouyx/7089914/webrev.05/ .

The reason for checking high contrast is windows allow
"win.3d.backgroundColor" to
be set black in non-high contrast mode.

The testcase is also cleared. Please review again.

On Fri, Feb 10, 2012 at 10:36 PM, Pavel Porvatov
<pavel.porvatov at oracle.com>wrote:

>  Hi Sean,
>
> Hi Pavel,
>
>     I modified the patch again. It's now at
> http://cr.openjdk.java.net/~zhouyx/7089914/webrev.03/ .
> 3/4 comments are covered. And there are a little comment bellow, please
> have a look. Thanks.
>
>
> On Mon, Jan 16, 2012 at 6:46 PM, Pavel Porvatov <pavel.porvatov at oracle.com
> > wrote:
>
>>  Hi Sean,
>>
>> There are several comments for your patch:
>>
>> 1. I found one bug. In Win 7 there are several differnet High Contrast
>> themes. Under one of them ("High Contrast White") focus is not visible...
>>
>       This is fixed.
>
>
> Is there any reason to check high contrast mode in the WDesktopProperties
> class? I think in non-high contrast mode backgroundColor checking will be
> useful as well
>
>
>> 2. I also don't like synthetic property. Any desktop property has
>> listener (see com.sun.java.swing.plaf.windows.DesktopProperty#pcl) and
>> updated when correspondent value is changed. But your win.button.focusColor
>> property is updated only while full property reloading.
>>
>       I tried to not use DesktopProperty, but it is found that the
> modification becomes much more complex and not working well. On the other
> hand, I think
>       use DesktopProperty is reasonable: it is so much like a desktop
> property that the only difference is it can not be changed separately.
>
> What do you think about the following solution:
>
>     static class FocusColorProperty extends DesktopProperty {
>         FocusColorProperty(Object fallback) {
>             super("win.3d.backgroundColor", fallback);
>         }
>
>         @Override
>         protected Object configureValue(Object value) {
>             return Color.BLACK.equals(value) ? Color.WHITE : Color.BLACK;
>         }
>     }
>
> This class solves update problems. Actually I didn't check that but it
> seems it should work...
>
>
>
>
>>
>> 3. Please don't add references to bugs in the code. Everybody can trace
>> history of the code by VCS.
>>
>      Reference removed.
>
>>
>> 4. The changes in the class WindowsRadioButtonUI looks good. Is it
>> possible to make your TestButton test an automatic one and add it to the
>> fix?
>>
>      Test case added.
>
> Could you please clean the test from TODO and dead code (like commented
> code)
>
> Regards, Pavel
>
>
>
>>
>> Regards, Pavel
>>
>>
>>  Hi all,
>>
>>     This is for bug 7089914,
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7089914 .
>> OpenJDK uses black as focus color in windows LAF. However, in high
>> contrast mode, windows
>> uses white as focus color.
>>    In additional, the patch also modified WindowsRadioButtonUI.java so it
>> will reload its style
>> when system setting is changed.
>>
>>     The webrev link is :
>> http://cr.openjdk.java.net/~zhouyx/7089914/webrev.02/
>>
>>  The previous discussions are:
>>
>> http://mail.openjdk.java.net/pipermail/swing-dev/2011-September/001703.html
>> http://mail.openjdk.java.net/pipermail/swing-dev/2011-October/001794.html
>> http://mail.openjdk.java.net/pipermail/swing-dev/2011-December/001871.html
>>
>>     Any comments on the this version? thanks.
>>
>>
>>
>
>
>  --
> Best Regards,
> Sean Chou
>
>
>


-- 
Best Regards,
Sean Chou
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120215/48c9c938/attachment.html>