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.porvatov@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.porvatov@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
      
         
   ">

(original) (raw)

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.porvatov@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.porvatov@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.porvatov@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