Hi Sean,
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/
<http://cr.openjdk.java.net/%7Ezhouyx/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.
I modified your fix a little bit and rewrote the test (because it
contained concurrency problems, e.g. the passed field must be volatile
etc). Here is the commit:
http://hg.openjdk.java.net/jdk8/awt/jdk/rev/362867d5caa4
Thanks, Pavel
On Fri, Feb 10, 2012 at 10:36 PM, Pavel Porvatov
<[email protected] <mailto:[email protected]>> wrote:
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
<[email protected] <mailto:[email protected]>> 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
--
Best Regards,
Sean Chou
