<AWT Dev> Review Request for 8057574 : inconsistent behavior for setBackground (Windows/Linux)

[Sergey Bylokhov]

Looks fine, Thanks.

On 23.05.16 13:17, Prem Balakrishnan wrote:
> Hi Sergey,
>
> Fix for Panel Background scenario and test case updated(in ChildWindowProperties.java), updated as per review comments
> Please review the updated fix.
> http://cr.openjdk.java.net/~pkbalakr/8057574/webrev.05/
>
>
>>>   - There is other similar code like in WPanelPeer, please check in what situation the WPanelPeer get null from the getBackground/getForeground.
>>> It seems that WColor class is used only in this WPanelPeer.
>
> WPanelPeer initialize() method is called before setting default properties for windows(in WWindowPeer::initialize() method),
> Therefore getBackground/getForeground call in WPanelPeer::initialize() method returns NULL, if properties are not set explicitly.
> Hence null check cannot be remove. Replacing WColor will also impact the output visually.
>
> Regards,
> Prem
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Monday, May 23, 2016 12:41 PM
> To: Prem Balakrishnan; Ambarish Rapte; awt-dev at openjdk.java.net
> Subject: Re: Review Request for 8057574 : inconsistent behavior for setBackground (Windows/Linux)
>
> On 17.05.16 14:12, Prem Balakrishnan wrote:
>> You were right, setting SystemColor.control as default for all components will have visual impact on Window and Panel.
>
> I am not sure but in the latest version the color of the panel will not be changed if it was added to the window where the color was set:
>      Window w = new Frame();
>      w.setBackground(Color.GREEN);
>      Panel p = new Panel();
>      w.add(p);
>      w.pack();
>      w.dispose();
>      System.out.println("p.getBackground() = " + p.getBackground());
>
> The color of the panel should be green even if it was not set by the user. Can you confirm that it works properly after the fix? if not please fix and update the tests to catch this bug.
>
>> Only for Dialog default background color is set to SystemColor.control.
>> For window and Panel default background color is set to "WColor.WINDOW_BKGND".
>> There were no impact on Canvas and Button.
>>
>> Please review the updated patch.
>> In updated fix I have maintained the same default color for Dialog as well as other component(Window and Panel).
>>
>> http://cr.openjdk.java.net/~pkbalakr/8057574/webrev.04/
>>
>> Regards,
>> Prem
>>
>>
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Monday, May 16, 2016 5:25 PM
>> To: Prem Balakrishnan; Ambarish Rapte; awt-dev at openjdk.java.net
>> Subject: Re: Review Request for 8057574 : inconsistent behavior for
>> setBackground (Windows/Linux)
>>
>> Hi, Prem.
>> On 11.05.16 15:28, Prem Balakrishnan wrote:
>>> 1. Dialog background color set to SystemColor.control.
>>
>> Can you please confirm that background color for other components(Button, Canvas, Window for example) is not changed after this.
>>
>>>
>>> 2. WWindowPeer extends WPanelPeer, WPanelPeer Initialize() method is called Before we initialize default properties like background/foreground in WWindowPeer, hence getBackground/getForeground calls in WPanelPeer Initialize() method always returns NULL.
>>> I have updated the fix, by moving default initialization to the Base class(WPanelPeer).
>>
>> Do we need the new fields:defaultBackground and defaultForeground? why we cannot use SystemColor.xx directly?
>>
>>>
>>> Please review the updated fix.
>>> http://cr.openjdk.java.net/~pkbalakr/8057574/webrev.03/
>>>
>>> Regression and JCK tests passed without causing any regression with the Updated fix as well.
>>>
>>>
>>>
>>> Regards,
>>> Prem
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: Friday, April 29, 2016 7:28 PM
>>> To: Prem Balakrishnan; Ambarish Rapte; Semyon Sadetsky;
>>> awt-dev at openjdk.java.net
>>> Subject: Re: Review Request for 8057574 : inconsistent behavior for
>>> setBackground (Windows/Linux)
>>>
>>> Hi, Prem.
>>> Thanks for the new version, I just found some issues which I messed in the first version:
>>>   - It seems that after the fix the Dialog will use the different background color,(SystemColor.window instead of SystemColor.control).
>>>   - There is other similar code like in WPanelPeer, please check in what situation the WPanelPeer get null from the getBackground/getForeground.
>>> It seems that WColor class is used only in this WPanelPeer.
>>>
>>> On 29.04.16 8:06, Prem Balakrishnan wrote:
>>>> Hi Sergey,
>>>>
>>>> Thank you for the Review.
>>>> Update patch as per review comments.
>>>> http://cr.openjdk.java.net/~pkbalakr/8057574/webrev.01/
>>>>
>>>> Regards,
>>>> Prem
>>>>
>>>> -----Original Message-----
>>>> From: Sergey Bylokhov
>>>> Sent: Thursday, April 28, 2016 6:16 PM
>>>> To: Prem Balakrishnan; Semyon Sadetsky; Ambarish Rapte;
>>>> awt-dev at openjdk.java.net
>>>> Subject: Re: Review Request for 8057574 : inconsistent behavior for
>>>> setBackground (Windows/Linux)
>>>>
>>>> Hi, Prem.
>>>> The fix looks fine. But please preserve the comments in
>>>> XWindowPeer.java
>>>>
>>>> On 25.04.16 9:00, Prem Balakrishnan wrote:
>>>>> Hi*,*
>>>>>
>>>>> Please review fix for JDK9,
>>>>>
>>>>> *Bug:*https://bugs.openjdk.java.net/browse/JDK-8057574
>>>>>
>>>>> *Webrev:*http://cr.openjdk.java.net/~pkbalakr/8057574/webrev.00/
>>>>>
>>>>> *Issue:*
>>>>>
>>>>> inconsistent behavior for setBackground (Windows/Linux)
>>>>>
>>>>> Inconsistency also exists for Foreground and Font properties across
>>>>> platforms (Win and Linux)
>>>>>
>>>>> *Fix: *
>>>>>
>>>>> Uniform behaviour is maintained by making , child component NOT to
>>>>> inherit  parent properties (Background, Foreground and Font)
>>>>>
>>>>> across all the platforms(Win/Linux/Mac).
>>>>>
>>>>> *Regression and JCK tests passed without causing any regression
>>>>> with the suggested fix.*
>>>>>
>>>>> Regards,
>>>>> Prem
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>>>
>>>
>>>
>>> --
>>> Best regards, Sergey.
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
> --
> Best regards, Sergey.
>


-- 
Best regards, Sergey.