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

[Pavel Porvatov]

Hi Sean,
> Hi Pavel,
>
>    I modified the patch again. It's now at 
> http://cr.openjdk.java.net/~zhouyx/7089914/webrev.03/ 
> <http://cr.openjdk.java.net/%7Ezhouyx/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 <mailto: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/
>>     <http://cr.openjdk.java.net/%7Ezhouyx/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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120210/b1c8fdc4/attachment.html>