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