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.scherba...@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.bylok...@oracle.com <mailto:sergey.bylok...@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.bylok...@oracle.com >>>>>> <mailto:sergey.bylok...@oracle.com><mailto:sergey.bylok...@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. >>> >> >> >
