[rfc][icedtea-web] bringing applets out of browser (part2)
Jiri Vanek
jvanek at redhat.com
Thu Dec 18 16:13:36 UTC 2014
Here it is!
The ultimate maistrwerk :))
Yes. The applets run by javaws -html really run in applets sandbox! An no more hacks done! A lof of
them is now working (nrearly all, except all jaavscript one :(( )
My favourite is: ./javaws -html http://www.w3.org/People/mimasa/test/object/java/clock all :)))
I have cleaned the code a lot - now it can be compiled without plugin, but plugin.jar is need in
run-time, if -html is used (otherwise is not)
I smuggeld two refactorings inside - get rid of HashTable. I could not look into it anymore... And
remover Reflect class. It is not used. It can be removed as separate changeset without issues. But
please dont wont me to separate heshtable removal.
Except fact that it is buildable, now all (or selected) applets on page can be run. Docs will
probably need a bit of tuning.
Unluckily I had not fixed quite lot of your nits :(( See the replies nline.
btw - is there some voulenteer to test it O:) (like try to hack it? and add reproducers O:))
On 12/15/2014 09:23 PM, Jie Kang wrote:
> Hello,
>
>
> Review in-line with code below:
>
>
> diff -r b5fa37369ea3 netx/net/sourceforge/jnlp/MalformedXMLParser.java
> --- a/netx/net/sourceforge/jnlp/MalformedXMLParser.java Tue Dec 09 10:10:10 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/MalformedXMLParser.java Thu Dec 11 17:09:23 2014 +0100
> @@ -45,7 +45,6 @@
> [...]
>
> - private InputStream xmlizeInputStream(InputStream original) throws ParseException {
> + public static InputStream xmlizeInputStream(InputStream original) throws ParseException {
>
> This change concerns me. I can understand making it public, but why static as well? Can't a user construct an object and call the method from the object?
>
Why not? It is perfectly ok. This is utility method converting one stream to another. Bounded with
tagsoup optional dependence. afaik it is best what could be done with this method.
> In Parser.java we have:
>
> Object instance = klass.newInstance();
> Method m = klass.getMethod("getRootNode", InputStream.class);
>
> whereas in AppletExtracter.java we have:
>
> + Class<?> klass = Class.forName(Parser.MALFORMED_PARSER_CLASS);
> + Method m = klass.getMethod("xmlizeInputStream", InputStream.class);
>
> I think you should be consistent. Either instantiate a MalformedXMLParser in all cases, or don't instantiate one ever (make getRootNode static).
This is very god catch, unluckily on two different cases:
In parser, we are calling overridden method, to supply modified behavior to modified parser.
In my AppletExtracter i'm really only calling this method, without compile time dependence.
>
>
> diff -r b5fa37369ea3 netx/net/sourceforge/jnlp/NetxPanel.java
> --- a/netx/net/sourceforge/jnlp/NetxPanel.java Tue Dec 09 10:10:10 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/NetxPanel.java Thu Dec 11 17:09:23 2014 +0100
> @@ -67,9 +67,9 @@
>
> + public void init(PluginBridge bridge) throws LaunchException {
>
> Just wanted to say, I really like this refactor, it makes code easier to read.
:)
>
> diff -r b5fa37369ea3 netx/net/sourceforge/jnlp/PluginBridge.java
> --- a/netx/net/sourceforge/jnlp/PluginBridge.java Tue Dec 09 10:10:10 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/PluginBridge.java Thu Dec 11 17:09:23 2014 +0100
> @@ -49,7 +49,7 @@
> */
> public class PluginBridge extends JNLPFile {
>
> Something I've been thinking about for quite a while here is: we probably shouldn't extend JNLPFile, instead we should have an instance of JNLPFile inside the PluginBridge. It would make things much easier to extend when we add features and require more things from the JNLPFile to be used by the PluginBridge. Just a thought.
This would force creation of JNLPinterface, and both those classes to implement it. An plugin bridge
will contains jnlpfile instance anyway. I'm not sure if final result is worthy of the huge effort.
>
>
> diff -r b5fa37369ea3 netx/net/sourceforge/jnlp/runtime/AppletExtracter.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/runtime/AppletExtracter.java Thu Dec 11 17:09:23 2014 +0100
> @@ -0,0 +1,273 @@
>
> s/Extracter/Extractor/
fixed. Actually Ihave split this class to three smaller.
> + private static final String[] APPLETS = new String[]{
> + "applet", "APPLET", "Applet",
>
> Instead of containing a list of accepted formats for 'applet', shouldn't the parser just accept any form of 'applet'? Just take whatever input string is and make it all lower-case, then check if it is the same as 'applet'. Is it because elements such as 'APPLet' not acceptable?
I wish so!
However I dont know any way hw to do case-insensitive match on atributes on sax-compliant parsers.
Instead of hardcoded list, I can do some permutations but afaik this should be enough....
>
>
> + private static class AppletParser {
> [...]
> + public AppletParser(Element applet, URL docbase) {
> + this.source = applet;
>
> Why the difference in field name and constructor argument name? I think they should be the same.
Well, this was intentional :)
The applet element is passed in, where it is serving as source element for any other processing... kept.
>
> Can you also add some comments to what AppletParser is parsing from and what it is parsing into?
done.
>
> + private URL createCodebase() throws ParseException, MalformedURLException {
> + String written = source.getAttribute("codebase");
>
> Instead of String written, how about String codebase?
> diff -r b5fa37369ea3 netx/net/sourceforge/jnlp/runtime/Boot.java
> --- a/netx/net/sourceforge/jnlp/runtime/Boot.java Tue Dec 09 10:10:10 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/runtime/Boot.java Thu Dec 11 17:09:23 2014 +0100
> @@ -16,23 +16,30 @@
> [...]
> +// int width = e.getComponent().getWidth();
> +// int height = e.getComponent().getHeight();
> +// np.updateSizeInAtts(height, width);
> +// np.setSize(1, 1);
> +// np.validate();
> +// np.setSize(width, height);
> +// np.validate();
> +// np.getApplet().resize(width, height);
> +// np.getApplet().validate();
>
> Please remove this code if it's no longer needed.
Not exactly. It is the code copypasted from in-browser resize. In time of writing I had notestcase
where to try it. So I would notremove - but - in meantime i made all jogamp and similar appelts run
and found that resizing is working perfectl fien even with my appletcontext stub. So removed even
with listener.
>
> diff -r b5fa37369ea3 plugin/icedteanp/java/sun/applet/PluginMain.java
> --- a/plugin/icedteanp/java/sun/applet/PluginMain.java Tue Dec 09 10:10:10 2014 +0100
> +++ b/plugin/icedteanp/java/sun/applet/PluginMain.java Thu Dec 11 17:09:23 2014 +0100
> @@ -84,7 +84,6 @@
> [...]
> + initSecurityContext(streamHandler);
> +
>
> PluginAppletViewer.setStreamhandler(streamHandler);
>
> I think there might be extra spaces above.
removed.
>
>
> diff -r b5fa37369ea3 plugin/icedteanp/java/sun/applet/PluginStreamHandler.java
> --- a/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java Tue Dec 09 10:10:10 2014 +0100
> +++ b/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java Thu Dec 11 17:09:23 2014 +0100
> @@ -53,6 +53,20 @@
> + public static final PluginStreamHandler DummyHandler = new PluginStreamHandler(new InputStream() {
> +
> + @Override
> + public int read() throws IOException {
> + return 0;
> + }
>
>
> The line spacing here makes it a little awkward to read. Can you put the 'new InputStream() {' on a new line as well?
yaaah!!! I was wondering why it looked so nasty :DD Fixed!
>
> + }, new OutputStream() {
> +
> + @Override
> + public void write(int b) throws IOException {
> + //
> + }
> + });
>
>
> So for above it would become:
>
> + public static final PluginStreamHandler DummyHandler = new PluginStreamHandler(
> + new InputStream() {
> + @Override
> + public int read() throws IOException {
> + return 0;
> + }
> + }, new OutputStream() {
> + @Override
> + public void write(int b) throws IOException {
> + //
> + }
> + });
>
>
> Looks nice so far. Will continue manual testing and if I find any errors I will let you know.
Thank you for check!
Sorry for not obeying:(
-------------- next part --------------
A non-text attachment was scrubbed...
Name: appletsOutOfBrowserShortcuts-head-06.patch
Type: text/x-patch
Size: 86400 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20141218/278d1c74/appletsOutOfBrowserShortcuts-head-06-0001.patch>
More information about the distro-pkg-dev
mailing list