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

[Sergey Bylokhov]

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


-- 
Best regards, Sergey.