[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-awt-dev
Subject: Re: <AWT Dev> Review Request for 8057574 : inconsistent behavior for setBackground (Windows/Linux)
From: Sergey Bylokhov <Sergey.Bylokhov () oracle ! com>
Date: 2016-05-23 10:29:10
Message-ID: a3cd89fe-ca77-6d7b-4046-00704557209e () oracle ! com
[Download RAW message or body]
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@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@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@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@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.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic