On Mar 5, 2016, at 10:54 AM, David DeHaven <david.deha...@oracle.com> wrote: > > > I've updated the webrev, hopefully one last time with feedback from Joe Darcy. > http://cr.openjdk.java.net/~ddehaven/8132743/webrev.2/ > > - Relocated package Javadoc to above the package declaration in > package-info.java > - Reworded the Javadoc on the default JSObject ctor >
Looks fine. Mandy > A point was brought up that the default ctor could probably be > package-private, but there's no time to investigate what the impact would be > so I will file a follow-up issue to investigate doing that at a later date. > > -DrD- > >> On Mar 4, 2016, at 7:44 AM, Kevin Rushforth <kevin.rushfo...@oracle.com> >> wrote: >> >> Looks good to me. >> >> -- Kevin >> >> >> David DeHaven wrote: >>> Updated webrev: >>> >>> http://cr.openjdk.java.net/~ddehaven/8132743/webrev.1/ >>> >>> >>> Added jdk.jsobject to MAIN_MODULES >>> Made suggested Javadoc changes >>> Gave JSException a real serialVersionUID >>> >>> -DrD- >>> >>> >>> >>> >>>> On Mar 3, 2016, at 5:06 PM, David DeHaven <david.deha...@oracle.com> >>>> wrote: >>>> >>>> >>>> Adding it to MAIN_MODULES, I now see it in bootmodules.jimage: >>>> /jdk.jsobject/jdk/internal/netscape/javascript/spi/JSObjectProvider.class >>>> /jdk.jsobject/netscape/javascript/JSException.class >>>> /jdk.jsobject/netscape/javascript/JSObject$ProviderLoader$1.class >>>> /jdk.jsobject/netscape/javascript/JSObject$ProviderLoader.class >>>> /jdk.jsobject/netscape/javascript/JSObject.class >>>> >>>> -DrD- >>>> >>>> >>>> >>>>> jdk9-dev is not the right mailing list. I bcc’ed it and add jigsaw-dev >>>>> instead. >>>>> >>>>> >>>>> >>>>> >>>>>> On Mar 3, 2016, at 3:57 PM, Kevin Rushforth <kevin.rushfo...@oracle.com> >>>>>> wrote: >>>>>> >>>>>> Looks OK to me. I did a quick test build and I can see the new package >>>>>> in the exploded JDK, but not in the images. Maybe I did something wrong? >>>>> Good catch. >>>>> >>>>> jdk.jsobject needs to be added in MAIN_MODULES list in make/Images.gmk >>>>> >>>>> Mandy >>>>> >>>>> >>>>> >>>>>> -- Kevin >>>>>> >>>>>> >>>>>> David DeHaven wrote: >>>>>> >>>>>> >>>>>>>>> JBS Issue: >>>>>>>>> >>>>>>>>> >>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8132743 >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Code review: >>>>>>>>> >>>>>>>>> >>>>>>>>> http://cr.openjdk.java.net/~ddehaven/8132743/webrev.0/ >>>>>>>> Looks okay. There is no @since - I guess it’s okay because >>>>>>>> netscape.javascript has been shipped with plugin for long time. >>>>>>> I can't track down when it was first included. It pre-dates anything >>>>>>> I've looked at so far. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>>> package-info.java >>>>>>>> "when running in an {@link java.applet.Applet Applet}” - is this true >>>>>>>> when running with JavaFX webkit? >>>>>>> Yes, I believe so, assuming you have a JSObject representing the root >>>>>>> window object. Maybe that should be reworded, I think just remove the >>>>>>> "when running in an Applet" part. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>>> JSObject.java >>>>>>>> @throws JSException is missing in the methods >>>>>>>> >>>>>>>> Does it throw NPE if the parameter is null? Or JSException - that >>>>>>>> needs to be specified. >>>>>>>> >>>>>>>> Nit: it’d be good to wrap null with {@code null} in the javadoc. >>>>>>> Ok. I can fix that. >>>>>>> >>>>>>> -DrD- >
