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

[Avik Niyogi]

Hi All,
Thank you for the reviews. Kindly request to commit the changes for the same.
With Regards,
Avik Niyogi

> On 12-Jan-2016, at 11:23 pm, Alexander Scherbatiy <alexandr.scherbatiy at oracle.com> wrote:
> 
> The fix looks good to me.
> 
> Thanks,
> Alexandr.
> 
> On 12/01/16 17:53, Sergey Bylokhov wrote:
>> Looks fine.
>> 
>> On 04/01/16 12:41, Avik Niyogi wrote:
>>> Hi All,
>>> Please review the following incorporated with inputs provided
>>> http://cr.openjdk.java.net/~aniyogi/8041894/webrev.01/
>>> 
>>> With Regards,
>>> Avik Niyogi
>>>> On 28-Dec-2015, at 7:05 pm, Sergey Bylokhov
>>>> <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>> wrote:
>>>> 
>>>> Hi, Avik
>>>> On 21/12/15 07:17, Avik Niyogi wrote:
>>>>> Hi Sergey,
>>>>> I verified that the issue is only with Aqua Look and feel and not other
>>>>> look and feels.
>>>> 
>>>> It will be better to prove it in the test, so the bug will not occur
>>>> again in some other/new L&F.
>>>> 
>>>>> 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,
>>>> 
>>>> This is unrelated to the code style. The cases which you selected are
>>>> necessary because we pass these values to the replaceEditor(), note
>>>> that parameters of replaceEditor are JComponent, so we must cast in
>>>> these cases. Same for (ComponentOrientation) e.getNewValue() because
>>>> we pass it to applyComponentOrientation(ComponentOrientation), but in
>>>> case of getOldValue it is not necessary because we compare it to null
>>>> only.
>>>> 
>>>> Also please do not remove the empty line before package com.apple.laf;
>>>> 
>>>>> 
>>>>> 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
>>>>>> <Sergey.Bylokhov at oracle.com
>>>>>> <mailto:Sergey.Bylokhov at oracle.com><mailto: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.
>>>>> 
>>>> 
>>>> 
>>>> -- 
>>>> Best regards, Sergey.
>>> 
>> 
>> 
>