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

[Alexander Scherbatiy]

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.
>>
>
>