Hi All, Gentle reminder: Please provide review and provide +1 for the review in case it is acceptable. Thank you in advance. With Regards, Avik Niyogi
> On 04-Jan-2016, at 3:11 pm, Avik Niyogi <avik.niy...@oracle.com> wrote: > > Hi All, > Please review the following incorporated with inputs provided > http://cr.openjdk.java.net/~aniyogi/8041894/webrev.01/ > <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 <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 >>>>> <https://bugs.openjdk.java.net/browse/JDK-8041894> >>>>> >>>>> *Webrev*: http://cr.openjdk.java.net/~aniyogi/8041894/webrev.00/ >>>>> <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. >
