<Swing Dev> Review request for 8041894: Test javax/swing/JSpinner/8008657/bug8008657.java failed on Mac

[Rajeev Chamyal]

Fix looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 21 December 2015 09:47
To: Sergey Bylokhov
Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> Review request for 8041894: Test javax/swing/JSpinner/8008657/bug8008657.java failed on Mac

 

Hi Sergey,

I verified that the issue is only with Aqua Look and feel and not other look and feels.

The type cast for ComponentOrientation was done in the code just for two reasons:

1) User readability for the component within the event e.

2) The cast for which type it is specified. For example, it can be noted that in the older code,

 

if ("editor".equals(propertyName)) {

                    final JComponent oldEditor = (JComponent) e.getOldValue();

                    final JComponent newEditor = (JComponent) e.getNewValue();

                    ui.replaceEditor(oldEditor, newEditor);

                    ui.updateEnabledState();

                } else if ("componentOrientation".equals(propertyName)) {

                    ComponentOrientation o

                            = (ComponentOrientation) e.getNewValue();

                    if (o != (ComponentOrientation) e.getOldValue()) {

                        JComponent editor = spinner.getEditor();

                        if (editor != null) {

                            editor.applyComponentOrientation(o);

                        }

                        spinner.revalidate();

                        spinner.repaint();

                    }

Casting of JComponent is done explicitly for the values there. Maintaining same standard 

and scoping out uncertainty seems correct in the context.

 

With Regards,

Avik Niyogi

 

On 19-Dec-2015, at 4:47 am, Sergey Bylokhov <HYPERLINK "mailto:Sergey.Bylokhov at oracle.com"Sergey.Bylokhov at oracle.com> wrote:

 

Hi, Avik.
Looks fine to me. It seems that the cast here is not necessary?
if (o != (ComponentOrientation) e.getOldValue())

Can you confirm that this bug not affectes our other looks and feels? It will be good to update the test to prove that.

On 17/12/15 10:21, Avik Niyogi wrote:





Hi All,

Kindly review the fix for JDK9.
*Bug*: https://bugs.openjdk.java.net/browse/JDK-8041894

*Webrev*: http://cr.openjdk.java.net/~aniyogi/8041894/webrev.00/

*Issue*: Test case throws an exception as subcomponents of were not
getting orientation
 property assignment.

*Cause*: The case where property name as Component orientation not
present to traverse
the property to subcomponents of spinner for Aqua look and feel.

*Fix*: The else if clause for this property name was added while
checking for for editor to
take charge of applying the component orientation to all subsequent
subcomponents.

With Regards,
Avik Niyogi



-- 
Best regards, Sergey.

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20151221/208237ae/attachment.html>