[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: Semyon Sadetsky <semyon.sadetsky () oracle ! com>
Date: 2016-01-21 17:54:57
Message-ID: 56A11B71.7080603 () oracle ! com
[Download RAW message or body]
On 1/21/2016 3:55 PM, Sergey Bylokhov wrote:
> On 21/01/16 09:42, Semyon Sadetsky wrote:
> >
> >
> > On 1/21/2016 4:05 AM, Sergey Bylokhov wrote:
> > > On 19/01/16 12:12, Semyon Sadetsky wrote:
> > > > Hi Prem,
> > > >
> > > > I would choose to inherit all them since the javadoc states that.
> > >
> > > There are more issue:
> > > - The javadoc of Compoenent.getBackground() states that the color of
> > > the parent container should be returned, but the window has not parent
> > > container it has the owner. Moreover the javadoc for
> > > Window.getBackground() simply states that it returns the color of the
> > > window.
> > What I see in the code:
> > Component.java 1812:
> > /**
> > * Gets the background color of this component.
> > * @return this component's background color; if this component
> > does
> > * not have a background color,
> > * the background color of its parent is returned
> > * @see #setBackground
> > * @since 1.0
> > */
> > @Transient
> > public Color getBackground() {
> > -------
> > No *container* word in java doc, simply "the background color of its
> > parent".
>
> The parent in terms of our api is "parent container".
> /**
> * Gets the parent of this component.
> * @return the parent container of this component
> * @since 1.0
> */
> public Container getParent() {
> }
>
What you are explaining is not clear from above docs. In general owner
and parent have different meaning. Owner is only responsible for the
owned window resources deallocation while the parent window controls
appearance of its child windows. In java windows we have both roles
combined. In native platforms they are separated.
Then why the background is not inherited and foreground and font should
be inherited on Windows platform? Should Linux and OS X be aligned to
this understanding?
>
> >
> > Window.java 623:
> > public Window(Window owner, GraphicsConfiguration gc) {
> > this(gc);
> > ownedInit(owner);
> > }
> >
> > private void ownedInit(Window owner) {
> > this.parent = owner;
> > -----------
> > Owned window *does* have a parent.
> >
> > Window.java 3758:
> > public Color getBackground() {
> > return super.getBackground();
> > }
> > ------
> > The super is Component.getBackground(), so the same logic is used for
> > background color definition.
>
> But the specification is different.
>
> >
> > > javadoc for Window.getBackground() simply states that it returns the
> > color of the window
> > It does, but the logic that defines this color takes parent's color in
> > case of null. No discrepancy.
>
> Actually no, it was intentionally changed on windows in 96 for Window
> and since 99 for Dialog. On osx it works from the beginning. The only
> platform which works differently is linux.
>
> >
> > > - If we state that the parent/owner color should be used will mean
> > > that isBackgroundSet() for our window should return false. And the
> > > owner color should be returned when its color was changed after our
> > > window was shown. But I assume that if our window is shown the changes
> > > in the owner color does not affect us.
> > >
> > > My assumptions is that we should set it, if it was not set by the
> > > user: At least even on windows we have such comments:
> > > WComponentPeer.java: // Set background color in C++, to avoid
> > > inheriting a parent's color.
> > > WDialogPeer.java: //* Native create() peeks at target's background
> > > and if it's null calls this method to arrage for default background to
> > > be set on target. Can't make the check in Java, since getBackground
> > > will return owner's background if target has none set. */
> > >
> > >
> > >
> > > > But this may cause a compatibility issue as well as in the reversed
> > > > case
> > > > (to not inherit all).
> > > > Looks like the issue is not as easy as it appeared.
> > > >
> > > > --Semyon
> > > >
> > > > On 1/19/2016 11:10 AM, Prem Balakrishnan wrote:
> > > > >
> > > > > Hi Semyon,
> > > > >
> > > > > Ideally what you said is right.
> > > > >
> > > > > Behaviors are different in different platform, as follows:
> > > > >
> > > > > *In Windows:*
> > > > >
> > > > > *B*ackground color *Doesn’t* Inherit Parent’s Background Color.
> > > > >
> > > > > Foreground color *Does* Inherit Parent’s Foreground Color.
> > > > >
> > > > > Font *Does* inherit Parent Font.
> > > > >
> > > > > *In Linux:*
> > > > >
> > > > > Background color *Does* Inherit Parent’s Background Color(Before my
> > > > > Fix)
> > > > >
> > > > > Foreground color *Does* inherit parent’s Foreground Color.
> > > > >
> > > > > Font *Does* inherit parent’s font.
> > > > >
> > > > > *In Mac:*
> > > > >
> > > > > Background color *Doesn’t* Inherit Parent’s Background Color
> > > > >
> > > > > Foreground color *Doesn’t* Inherit Parent’s Foreground Color.
> > > > >
> > > > > Font *Doesn’t* inherit parent’s font.
> > > > >
> > > > > Decision to be taken what should be the default scenario and
> > > > > appropriately fix others.
> > > > >
> > > > > Regards,
> > > > > Prem
> > > > >
> > > > > *From:*Semyon Sadetsky
> > > > > *Sent:* Tuesday, January 19, 2016 12:09 PM
> > > > > *To:* Prem Balakrishnan; Ambarish Rapte; Sergey Bylokhov;
> > > > > awt-dev@openjdk.java.net
> > > > > *Subject:* Re: Review Request for 8057574: Inconsistent behavior for
> > > > > setBackground (Windows/Linux)
> > > > >
> > > > > On 1/19/2016 6:30 AM, Prem Balakrishnan wrote:
> > > > >
> > > > > Hi Semyon,
> > > > >
> > > > > In the case of Foreground color, Behavior is same in Windows and
> > > > > Linux platforms.
> > > > >
> > > > > (i.e., Child inherits parents Foreground color).
> > > > >
> > > > > And from my point of view the behavior can be retained.
> > > > >
> > > > > That seems to me inconsistent when the background color is inherited
> > > > > and the foreground is not. I think they both should be either
> > > > > inherited either both not inherited.
> > > > >
> > > > > --Semyon
> > > > >
> > > > > Regards,
> > > > >
> > > > > Prem
> > > > >
> > > > > *From:* Semyon Sadetsky
> > > > > *Sent:* Monday, January 18, 2016 2:30 PM
> > > > > *To:* Prem Balakrishnan; Ambarish Rapte; Sergey Bylokhov;
> > > > > awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>
> > > > > *Subject:* Re: Review Request for 8057574: Inconsistent behavior for
> > > > > setBackground (Windows/Linux)
> > > > >
> > > > > Hi Prem,
> > > > >
> > > > > The updated version looks better.
> > > > > Should we do the same for the foreground color?
> > > > >
> > > > > --Semyon
> > > > >
> > > > > On 1/13/2016 4:24 PM, Prem Balakrishnan wrote:
> > > > >
> > > > > Hi Semyon,
> > > > >
> > > > > The scenario which you mentioned, child Window with Frame/Dialog
> > > > > as parent *FAILED*.
> > > > >
> > > > > I have fixed the issue and updated relative test cases .
> > > > >
> > > > > *Updated Webrev:
> > > > > *<http://cr.openjdk.java.net/%7Earapte/prem/8057574/webrev.02/>http://cr.openjdk.java.net/~arapte/prem/8057574/webrev.02/ \
> > > > >
> > > > >
> > > > > Regards,
> > > > >
> > > > > Prem
> > > > >
> > > > > *From:* Semyon Sadetsky
> > > > > *Sent:* Wednesday, January 13, 2016 4:13 PM
> > > > > *To:* Prem Balakrishnan; Ambarish Rapte; Sergey Bylokhov;
> > > > > awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>
> > > > > *Subject:* Re: Review Request for 8057574: Inconsistent behavior
> > > > > for setBackground (Windows/Linux)
> > > > >
> > > > >
> > > > > On 1/13/2016 1:24 PM, Prem Balakrishnan wrote:
> > > > >
> > > > > Hi Semyon,
> > > > >
> > > > > Yup tried with Window:
> > > > >
> > > > > Dialog checks parent’s type internally, if the type is Window
> > > > > following exception is thrown.
> > > > >
> > > > > I meant *child* Window with Frame or Dialog parent.
> > > > >
> > > > > Exception in thread "main" java.lang.IllegalArgumentException:
> > > > > Wrong parent window
> > > > >
> > > > > *Code snippet from awt/Dialog.java *
> > > > >
> > > > > public Dialog(Window owner, String title, ModalityType
> > > > > modalityType) {
> > > > >
> > > > > super(owner);
> > > > >
> > > > > if ((owner != null) &&
> > > > >
> > > > > !(owner instanceof Frame) &&
> > > > >
> > > > > !(owner instanceof Dialog))
> > > > >
> > > > > {
> > > > >
> > > > > throw new IllegalArgumentException("Wrong parent
> > > > > window");
> > > > >
> > > > > }
> > > > >
> > > > > …….
> > > > >
> > > > > …….
> > > > >
> > > > > Regards,
> > > > > Prem
> > > > >
> > > > > *From:* Semyon Sadetsky
> > > > > *Sent:* Wednesday, January 13, 2016 3:40 PM
> > > > > *To:* Prem Balakrishnan; Ambarish Rapte; Sergey Bylokhov;
> > > > > awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>
> > > > > *Subject:* Re: Review Request for 8057574: Inconsistent behavior
> > > > > for setBackground (Windows/Linux)
> > > > >
> > > > > Hi,
> > > > >
> > > > > On 1/13/2016 12:20 PM, Prem Balakrishnan wrote:
> > > > >
> > > > > Hi Semyon,
> > > > >
> > > > > 1. Patch is updated with the relative test for the fix.
> > > > >
> > > > > *Updated Webrev:*
> > > > > <http://cr.openjdk.java.net/%7Earapte/prem/8057574/webrev.01/>http://cr.openjdk.java.net/~arapte/prem/8057574/webrev.01/ \
> > > > >
> > > > >
> > > > > *Bug:*
> > > > > <https://bugs.openjdk.java.net/browse/JDK-8057574>https://bugs.openjdk.java.net/browse/JDK-8057574 \
> > > > >
> > > > >
> > > > >
> > > > > 2. The issue is observed when Frame is parent and
> > > > > Dialog
> > > > > is Child. (Scenario reported by Alexander Stepanov)
> > > > >
> > > > > The above issue is resolved with the same fix.
> > > > >
> > > > > Did you try Window child?
> > > > >
> > > > > --Semyon
> > > > >
> > > > >
> > > > > Test related to this scenario is also updated with the patch.
> > > > >
> > > > > Regards,
> > > > >
> > > > > Prem
> > > > >
> > > > > *From:* Semyon Sadetsky
> > > > > *Sent:* Monday, January 11, 2016 6:00 PM
> > > > > *To:* Prem Balakrishnan; Ambarish Rapte;
> > > > > <mailto:awt-dev@openjdk.java.net>awt-dev@openjdk.java.net
> > > > > *Subject:* Re: Review Request for 8057574: Inconsistent behavior
> > > > > for setBackground (Windows/Linux)
> > > > >
> > > > > On 1/11/2016 2:18 PM, Prem Balakrishnan wrote:
> > > > >
> > > > >
> > > > >
> > > > > Hi Semyon,
> > > > >
> > > > > Thanks for the review comments.
> > > > >
> > > > > 1. I will add a relative test for the fix.
> > > > >
> > > > > 2. The same issue is observed with Frame also,
> > > > >
> > > > > As the original issue is reported for dialog, fix
> > > > > submitted only for dialog.
> > > > >
> > > > > Should I combine fix for Dialog and Frame in the same patch?
> > > > >
> > > > > Yes. I would consider it as the same issue. All window types may
> > > > > have this issue.
> > > > >
> > > > > --Semyon
> > > > >
> > > > >
> > > > >
> > > > > Regards,
> > > > >
> > > > > Prem
> > > > >
> > > > > *From:* Semyon Sadetsky
> > > > > *Sent:* Monday, January 11, 2016 3:32 PM
> > > > > *To:* Prem Balakrishnan; Ambarish Rapte;
> > > > > <mailto:awt-dev@openjdk.java.net>awt-dev@openjdk.java.net
> > > > > *Subject:* Re: Review Request for 8057574: Inconsistent behavior
> > > > > for setBackground (Windows/Linux)
> > > > >
> > > > > Hi Prem,
> > > > >
> > > > > By default a fix requires a regression test which fails before
> > > > > the
> > > > > fix an passes after. When it is absent noreg-??? label should be
> > > > > set to the JIRA ticket. See
> > > > > <http://openjdk.java.net/guide/changePlanning.html>http://openjdk.java.net/guide/changePlanning.html \
> > > > >
> > > > > for possible noreg suffixes.
> > > > > But it seems to me that reg test can be written for the issue.
> > > > >
> > > > > In JIRA Alexander Stepanov noted that the issue takes place for
> > > > > Frame as well but your solution is fixing the Dialog only. Did
> > > > > you
> > > > > run the scenario with Frame?
> > > > >
> > > > > --Semyon
> > > > >
> > > > > On 1/4/2016 11:28 AM, Prem Balakrishnan wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > Please review fix for JDK9,
> > > > >
> > > > > *Bug:*
> > > > > <https://bugs.openjdk.java.net/browse/JDK-8057574>https://bugs.openjdk.java.net/browse/JDK-8057574 \
> > > > >
> > > > >
> > > > >
> > > > > *Webrev:*
> > > > > <http://cr.openjdk.java.net/%7Earapte/prem/8057574/webrev.00/>http://cr.openjdk.java.net/~arapte/prem/8057574/webrev.00/ \
> > > > >
> > > > >
> > > > > *Issue:*
> > > > >
> > > > > It is Platform specific Issue - LINUX
> > > > >
> > > > > If Child Dialog’s Background color is NOT set explicitly,
> > > > > Child Dialog inherits Parent Dialog’s Background Color.
> > > > >
> > > > > (In Windows OS Child Dialog is NOT inheriting parent Dialog’s
> > > > > Background Color)
> > > > >
> > > > > *Cause:*
> > > > >
> > > > > Default Background color for Dialog is not initialized.
> > > > >
> > > > > *Fix:*
> > > > >
> > > > > If Background color is not set explicitly , default
> > > > > Background
> > > > > color is set at the time of Dialog’s initialization.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Prem
> > > > >
> > > >
> > >
> > >
> >
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic