Reviewer needed - fix for Re: Strange behaviour during javaws -about
Dr Andrew John Hughes
ahughes at redhat.com
Thu Feb 24 09:04:31 PST 2011
On 17:43 Thu 24 Feb , Jiri Vanek wrote:
> On 02/24/2011 12:36 AM, Omair Majid wrote:
>
> So, this patch expects fixed about.jnlp to have code base of
> file://$DEST_DIR/share
>
> Then removes all-permissions, and lunched fixed window without them. The
> window is not running in invoke later(which probably caused the hang)
> but runs in javaws's own thread. Also points directly to correct jnlp
> local file and runs offline.
>
Comments inline.
Please separate any formatting changes to another patch, though I don't
see why these are necessary at all.
> > On 02/23/2011 12:51 PM, Dr Andrew John Hughes wrote:
> >> What is the motivation for this change? Assuming it still runs without
> >> all-permissions, the change looks fine.
> >>
> >
> > From my very limited testing, it doesnt :(
> >
> > snip
>
> # HG changeset patch
> # User Jiri Vanek <jvanek at redhat.com>
> # Date 1298565203 -3600
> # Node ID e2f1a23ade0931e1b6810cb06d4b313ee9333415
> # Parent d7fee305bd4f63d3a37cfa041b9af726e296d22f
> about.jar can now be lunched localy, without all-permissions, and
> about.jnlp is red from directory instead from jar.
>
> diff -r d7fee305bd4f -r e2f1a23ade09 ChangeLog
> --- a/ChangeLog Wed Feb 23 13:37:10 2011 -0500
> +++ b/ChangeLog Thu Feb 24 17:33:23 2011 +0100
> @@ -1,3 +1,11 @@
> +2011-02-24 Jiri Vanek <jvanek at redhat.com>
> +
> + * netx/net/sourceforge/jnlp/runtime/Boot.java: getAboutFile changed to
> return path to local about.jnlp instead of inner-ftom jar
What does ftom stand for?
> + * extras/net/sourceforge/jnlp/about/Main.java: removed ivokelater,
> useless hypertextlistener and runtime which needed special permissions.
Why is it useless?
Both this and the above need to be more verbose to include the changes made.
> + * netx/net/sourceforge/jnlp/resources/about.jnlp: removed
> <all-permissions>
> +
> +
> +
> 2011-02-23 Omair Majid <omajid at redhat.com>
>
> * Makefile.am: Add missing slash to JRE.
> diff -r d7fee305bd4f -r e2f1a23ade09
> extra/net/sourceforge/jnlp/about/Main.java
> --- a/extra/net/sourceforge/jnlp/about/Main.java Wed Feb 23 13:37:10
> 2011 -0500
> +++ b/extra/net/sourceforge/jnlp/about/Main.java Thu Feb 24 17:33:23
> 2011 +0100
> @@ -41,19 +41,15 @@
> import java.awt.Dimension;
> import java.awt.Toolkit;
> import java.io.IOException;
> -import java.net.URL;
> +
>
> import javax.swing.JFrame;
> import javax.swing.JPanel;
> import javax.swing.JTabbedPane;
> import javax.swing.UIManager;
> -import javax.swing.event.HyperlinkEvent;
> -import javax.swing.event.HyperlinkListener;
>
> -import net.sourceforge.jnlp.Launcher;
> -import net.sourceforge.jnlp.runtime.JNLPRuntime;
>
> -public class Main extends JPanel implements HyperlinkListener {
> +public class Main extends JPanel{
>
Why these changes?
> private final String notes =
> "/net/sourceforge/jnlp/about/resources/notes.html";
> private final String apps =
> "/net/sourceforge/jnlp/about/resources/applications.html";
> @@ -62,13 +58,11 @@
>
> public Main() throws IOException {
> super(new BorderLayout());
> -
> +
> HTMLPanel notesPanel = new HTMLPanel(getClass().getResource(notes));
> HTMLPanel appsPanel = new HTMLPanel(getClass().getResource(apps));
> HTMLPanel aboutPanel = new HTMLPanel(getClass().getResource(about));
>
> - appsPanel.pane.addHyperlinkListener(this);
> -
> tabbedPane = new JTabbedPane();
>
> tabbedPane.add("About NetX", aboutPanel);
> @@ -80,7 +74,6 @@
> }
>
> private static void createAndShowGUI() {
> - JNLPRuntime.setExitClass(Main.class);
>
> try {
> UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
> @@ -112,24 +105,8 @@
> }
>
> public static void main(String[] args) {
> - javax.swing.SwingUtilities.invokeLater(new Runnable() {
> - public void run() {
> - createAndShowGUI();
> - }
> - });
> - }
> + createAndShowGUI();
> + }
>
> - public void hyperlinkUpdate(HyperlinkEvent e) {
> - if (e.getEventType() == HyperlinkEvent.EventType.ACTIVATED) {
> - URL url = e.getURL();
>
> - Launcher launcher = new Launcher(
> - JNLPRuntime.getDefaultLaunchHandler());
> - try {
> - launcher.launchBackground(url);
> - } catch (Exception ex) {
> - ex.printStackTrace();
> - }
> - }
> - }
> }
> diff -r d7fee305bd4f -r e2f1a23ade09
> netx/net/sourceforge/jnlp/resources/about.jnlp
> --- a/netx/net/sourceforge/jnlp/resources/about.jnlp Wed Feb 23 13:37:10
> 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/resources/about.jnlp Thu Feb 24 17:33:23
> 2011 +0100
> @@ -1,5 +1,5 @@
> <?xml version="1.0" encoding="utf-8"?>
> -<jnlp spec="1.0" href="about.jnlp"
> codebase="http://icedtea.classpath.org/netx/">
> +<jnlp spec="1.0" href="about.jnlp"
> codebase="file:///home/jvanek/icedtea-web-image/share/icedtea-web/">
> <information>
> <title>About window for NetX</title>
> <vendor>NetX</vendor>
> @@ -12,7 +12,7 @@
> <jar href="about.jar"/>
> </resources>
> <security>
> - <all-permissions/>
> +
> </security>
> <application-desc main-class="net.sourceforge.jnlp.about.Main">
> </application-desc>
> diff -r d7fee305bd4f -r e2f1a23ade09
> netx/net/sourceforge/jnlp/runtime/Boot.java
> --- a/netx/net/sourceforge/jnlp/runtime/Boot.java Wed Feb 23 13:37:10
> 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/runtime/Boot.java Thu Feb 24 17:33:23
> 2011 +0100
> @@ -141,22 +141,26 @@
> System.exit(0);
> }
>
> - if (null != getOption("-about"))
> + if (null != getOption("-about")) {
> System.out.println(aboutMessage);
> + }
>
> - if (null != getOption("-verbose"))
> + if (null != getOption("-verbose")) {
> JNLPRuntime.setDebug(true);
> + }
>
I don't see the point in these changes. This is just noise which makes
the patch hard to read.
> if (null != getOption("-update")) {
> int value = Integer.parseInt(getOption("-update"));
> JNLPRuntime.setDefaultUpdatePolicy(new UpdatePolicy(value
> * 1000l));
> }
>
> - if (null != getOption("-headless"))
> + if (null != getOption("-headless")) {
> JNLPRuntime.setHeadless(true);
> + }
>
> - if (null != getOption("-noupdate"))
> + if (null != getOption("-noupdate")) {
> JNLPRuntime.setDefaultUpdatePolicy(UpdatePolicy.NEVER);
> + }
>
> if (null != getOption("-Xnofork")) {
> JNLPRuntime.setForksAllowed(false);
> @@ -194,8 +198,9 @@
> } catch (LaunchException ex) {
> // default handler prints this
> } catch (Exception ex) {
> - if (JNLPRuntime.isDebug())
> + if (JNLPRuntime.isDebug()) {
> ex.printStackTrace();
> + }
>
> fatalError(R("RUnexpected", ex.toString(),
> ex.getStackTrace()[0]));
> }
> @@ -218,8 +223,16 @@
> cl = ClassLoader.getSystemClassLoader();
> }
> try {
> - return
> cl.getResource("net/sourceforge/jnlp/resources/about.jnlp").toString();
> + //exctracts full path to about.jnlp
Typo; extracts
> + String s =
> cl.getResource("net/sourceforge/jnlp/runtime/Boot.class").toString();
> + s=s.substring(0,s.indexOf("!"));
> + s=s.substring(s.indexOf(":")+1);
> + s=s.substring(s.indexOf(":")+1);
> + s="file://"+s.replace("netx.jar","about.jnlp");
> + System.out.println(s);
> + return s;
What does this do?
> } catch (Exception e) {
> + e.printStackTrace();
> return null;
> }
> }
> @@ -235,8 +248,9 @@
> // override -jnlp with aboutFile
> if (getOption("-about") != null) {
> location = getAboutFile();
> - if (location == null)
> + if (location == null) {
> fatalError(R("RNoAboutJnlp"));
> + }
> } else {
> location = getJNLPFile();
> }
> @@ -246,21 +260,24 @@
> System.exit(1);
> }
>
> - if (JNLPRuntime.isDebug())
> + if (JNLPRuntime.isDebug()) {
> System.out.println(R("BFileLoc") + ": " + location);
> + }
>
> URL url = null;
>
> try {
> - if (new File(location).exists())
> - // TODO: Should be toURI().toURL()
> + if (new File(location).exists()) // TODO: Should be
> toURI().toURL()
> + {
> url = new File(location).toURL(); // Why use
> file.getCanonicalFile?
> - else
> + } else {
> url = new
> URL(ServiceUtil.getBasicService().getCodeBase(), location);
> + }
> } catch (Exception e) {
> fatalError("Invalid jnlp file " + location);
> - if (JNLPRuntime.isDebug())
> + if (JNLPRuntime.isDebug()) {
> e.printStackTrace();
> + }
> }
>
> boolean strict = (null != getOption("-strict"));
> @@ -275,20 +292,26 @@
> // add in extra params from command line
> addProperties(file);
>
> - if (file.isApplet())
> + if (file.isApplet()) {
> addParameters(file);
> + }
>
> - if (file.isApplication())
> + if (file.isApplication()) {
> addArguments(file);
> + }
>
> if (JNLPRuntime.isDebug()) {
> - if (getOption("-arg") != null)
> - if (file.isInstaller() || file.isApplet())
> + if (getOption("-arg") != null) {
> + if (file.isInstaller() || file.isApplet()) {
> System.out.println(R("BArgsNA"));
> + }
> + }
>
> - if (getOption("-param") != null)
> - if (file.isApplication())
> + if (getOption("-param") != null) {
> + if (file.isApplication()) {
> System.out.println(R("BParamNA"));
> + }
> + }
> }
>
> return file;
> @@ -304,8 +327,9 @@
> for (int i = 0; i < props.length; i++) {
> // allows empty property, not sure about validity of that.
> int equals = props[i].indexOf("=");
> - if (equals == -1)
> + if (equals == -1) {
> fatalError(R("BBadProp", props[i]));
> + }
>
> String key = props[i].substring(0, equals);
> String value = props[i].substring(equals + 1,
> props[i].length());
> @@ -325,8 +349,9 @@
> for (int i = 0; i < params.length; i++) {
> // allows empty param, not sure about validity of that.
> int equals = params[i].indexOf("=");
> - if (equals == -1)
> + if (equals == -1) {
> fatalError(R("BBadParam", params[i]));
> + }
>
> String name = params[i].substring(0, equals);
> String value = params[i].substring(equals + 1,
> params[i].length());
> @@ -381,10 +406,11 @@
> private static String getOption(String option) {
> String result[] = getOptions(option);
>
> - if (result.length == 0)
> + if (result.length == 0) {
> return null;
> - else
> + } else {
> return result[0];
> + }
> }
>
> /**
> @@ -398,14 +424,16 @@
>
> for (int i = 0; i < args.length; i++) {
> if (option.equals(args[i])) {
> - if (-1 == doubleArgs.indexOf(option))
> + if (-1 == doubleArgs.indexOf(option)) {
> result.add(option);
> - else if (i + 1 < args.length)
> + } else if (i + 1 < args.length) {
> result.add(args[i + 1]);
> + }
> }
>
> - if (args[i].startsWith("-") && -1 !=
> doubleArgs.indexOf(args[i]))
> + if (args[i].startsWith("-") && -1 !=
> doubleArgs.indexOf(args[i])) {
> i++;
> + }
> }
>
> return result.toArray(new String[result.size()]);
> # HG changeset patch
> # User Jiri Vanek <jvanek at redhat.com>
> # Date 1298565203 -3600
> # Node ID e2f1a23ade0931e1b6810cb06d4b313ee9333415
> # Parent d7fee305bd4f63d3a37cfa041b9af726e296d22f
> about.jar can now be lunched localy, without all-permissions, and about.jnlp is red from directory instead from jar.
>
> diff -r d7fee305bd4f -r e2f1a23ade09 ChangeLog
> --- a/ChangeLog Wed Feb 23 13:37:10 2011 -0500
> +++ b/ChangeLog Thu Feb 24 17:33:23 2011 +0100
> @@ -1,3 +1,11 @@
> +2011-02-24 Jiri Vanek <jvanek at redhat.com>
> +
> + * netx/net/sourceforge/jnlp/runtime/Boot.java: getAboutFile changed to return path to local about.jnlp instead of inner-ftom jar
> + * extras/net/sourceforge/jnlp/about/Main.java: removed ivokelater, useless hypertextlistener and runtime which needed special permissions.
> + * netx/net/sourceforge/jnlp/resources/about.jnlp: removed <all-permissions>
> +
> +
> +
> 2011-02-23 Omair Majid <omajid at redhat.com>
>
> * Makefile.am: Add missing slash to JRE.
> diff -r d7fee305bd4f -r e2f1a23ade09 extra/net/sourceforge/jnlp/about/Main.java
> --- a/extra/net/sourceforge/jnlp/about/Main.java Wed Feb 23 13:37:10 2011 -0500
> +++ b/extra/net/sourceforge/jnlp/about/Main.java Thu Feb 24 17:33:23 2011 +0100
> @@ -41,19 +41,15 @@
> import java.awt.Dimension;
> import java.awt.Toolkit;
> import java.io.IOException;
> -import java.net.URL;
> +
>
> import javax.swing.JFrame;
> import javax.swing.JPanel;
> import javax.swing.JTabbedPane;
> import javax.swing.UIManager;
> -import javax.swing.event.HyperlinkEvent;
> -import javax.swing.event.HyperlinkListener;
>
> -import net.sourceforge.jnlp.Launcher;
> -import net.sourceforge.jnlp.runtime.JNLPRuntime;
>
> -public class Main extends JPanel implements HyperlinkListener {
> +public class Main extends JPanel{
>
> private final String notes = "/net/sourceforge/jnlp/about/resources/notes.html";
> private final String apps = "/net/sourceforge/jnlp/about/resources/applications.html";
> @@ -62,13 +58,11 @@
>
> public Main() throws IOException {
> super(new BorderLayout());
> -
> +
> HTMLPanel notesPanel = new HTMLPanel(getClass().getResource(notes));
> HTMLPanel appsPanel = new HTMLPanel(getClass().getResource(apps));
> HTMLPanel aboutPanel = new HTMLPanel(getClass().getResource(about));
>
> - appsPanel.pane.addHyperlinkListener(this);
> -
> tabbedPane = new JTabbedPane();
>
> tabbedPane.add("About NetX", aboutPanel);
> @@ -80,7 +74,6 @@
> }
>
> private static void createAndShowGUI() {
> - JNLPRuntime.setExitClass(Main.class);
>
> try {
> UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
> @@ -112,24 +105,8 @@
> }
>
> public static void main(String[] args) {
> - javax.swing.SwingUtilities.invokeLater(new Runnable() {
> - public void run() {
> - createAndShowGUI();
> - }
> - });
> - }
> + createAndShowGUI();
> + }
>
> - public void hyperlinkUpdate(HyperlinkEvent e) {
> - if (e.getEventType() == HyperlinkEvent.EventType.ACTIVATED) {
> - URL url = e.getURL();
>
> - Launcher launcher = new Launcher(
> - JNLPRuntime.getDefaultLaunchHandler());
> - try {
> - launcher.launchBackground(url);
> - } catch (Exception ex) {
> - ex.printStackTrace();
> - }
> - }
> - }
> }
> diff -r d7fee305bd4f -r e2f1a23ade09 netx/net/sourceforge/jnlp/resources/about.jnlp
> --- a/netx/net/sourceforge/jnlp/resources/about.jnlp Wed Feb 23 13:37:10 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/resources/about.jnlp Thu Feb 24 17:33:23 2011 +0100
> @@ -1,5 +1,5 @@
> <?xml version="1.0" encoding="utf-8"?>
> -<jnlp spec="1.0" href="about.jnlp" codebase="http://icedtea.classpath.org/netx/">
> +<jnlp spec="1.0" href="about.jnlp" codebase="file:///home/jvanek/icedtea-web-image/share/icedtea-web/">
> <information>
> <title>About window for NetX</title>
> <vendor>NetX</vendor>
> @@ -12,7 +12,7 @@
> <jar href="about.jar"/>
> </resources>
> <security>
> - <all-permissions/>
> +
> </security>
> <application-desc main-class="net.sourceforge.jnlp.about.Main">
> </application-desc>
> diff -r d7fee305bd4f -r e2f1a23ade09 netx/net/sourceforge/jnlp/runtime/Boot.java
> --- a/netx/net/sourceforge/jnlp/runtime/Boot.java Wed Feb 23 13:37:10 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/runtime/Boot.java Thu Feb 24 17:33:23 2011 +0100
> @@ -141,22 +141,26 @@
> System.exit(0);
> }
>
> - if (null != getOption("-about"))
> + if (null != getOption("-about")) {
> System.out.println(aboutMessage);
> + }
>
> - if (null != getOption("-verbose"))
> + if (null != getOption("-verbose")) {
> JNLPRuntime.setDebug(true);
> + }
>
> if (null != getOption("-update")) {
> int value = Integer.parseInt(getOption("-update"));
> JNLPRuntime.setDefaultUpdatePolicy(new UpdatePolicy(value * 1000l));
> }
>
> - if (null != getOption("-headless"))
> + if (null != getOption("-headless")) {
> JNLPRuntime.setHeadless(true);
> + }
>
> - if (null != getOption("-noupdate"))
> + if (null != getOption("-noupdate")) {
> JNLPRuntime.setDefaultUpdatePolicy(UpdatePolicy.NEVER);
> + }
>
> if (null != getOption("-Xnofork")) {
> JNLPRuntime.setForksAllowed(false);
> @@ -194,8 +198,9 @@
> } catch (LaunchException ex) {
> // default handler prints this
> } catch (Exception ex) {
> - if (JNLPRuntime.isDebug())
> + if (JNLPRuntime.isDebug()) {
> ex.printStackTrace();
> + }
>
> fatalError(R("RUnexpected", ex.toString(), ex.getStackTrace()[0]));
> }
> @@ -218,8 +223,16 @@
> cl = ClassLoader.getSystemClassLoader();
> }
> try {
> - return cl.getResource("net/sourceforge/jnlp/resources/about.jnlp").toString();
> + //exctracts full path to about.jnlp
> + String s = cl.getResource("net/sourceforge/jnlp/runtime/Boot.class").toString();
> + s=s.substring(0,s.indexOf("!"));
> + s=s.substring(s.indexOf(":")+1);
> + s=s.substring(s.indexOf(":")+1);
> + s="file://"+s.replace("netx.jar","about.jnlp");
> + System.out.println(s);
> + return s;
> } catch (Exception e) {
> + e.printStackTrace();
> return null;
> }
> }
> @@ -235,8 +248,9 @@
> // override -jnlp with aboutFile
> if (getOption("-about") != null) {
> location = getAboutFile();
> - if (location == null)
> + if (location == null) {
> fatalError(R("RNoAboutJnlp"));
> + }
> } else {
> location = getJNLPFile();
> }
> @@ -246,21 +260,24 @@
> System.exit(1);
> }
>
> - if (JNLPRuntime.isDebug())
> + if (JNLPRuntime.isDebug()) {
> System.out.println(R("BFileLoc") + ": " + location);
> + }
>
> URL url = null;
>
> try {
> - if (new File(location).exists())
> - // TODO: Should be toURI().toURL()
> + if (new File(location).exists()) // TODO: Should be toURI().toURL()
> + {
> url = new File(location).toURL(); // Why use file.getCanonicalFile?
> - else
> + } else {
> url = new URL(ServiceUtil.getBasicService().getCodeBase(), location);
> + }
> } catch (Exception e) {
> fatalError("Invalid jnlp file " + location);
> - if (JNLPRuntime.isDebug())
> + if (JNLPRuntime.isDebug()) {
> e.printStackTrace();
> + }
> }
>
> boolean strict = (null != getOption("-strict"));
> @@ -275,20 +292,26 @@
> // add in extra params from command line
> addProperties(file);
>
> - if (file.isApplet())
> + if (file.isApplet()) {
> addParameters(file);
> + }
>
> - if (file.isApplication())
> + if (file.isApplication()) {
> addArguments(file);
> + }
>
> if (JNLPRuntime.isDebug()) {
> - if (getOption("-arg") != null)
> - if (file.isInstaller() || file.isApplet())
> + if (getOption("-arg") != null) {
> + if (file.isInstaller() || file.isApplet()) {
> System.out.println(R("BArgsNA"));
> + }
> + }
>
> - if (getOption("-param") != null)
> - if (file.isApplication())
> + if (getOption("-param") != null) {
> + if (file.isApplication()) {
> System.out.println(R("BParamNA"));
> + }
> + }
> }
>
> return file;
> @@ -304,8 +327,9 @@
> for (int i = 0; i < props.length; i++) {
> // allows empty property, not sure about validity of that.
> int equals = props[i].indexOf("=");
> - if (equals == -1)
> + if (equals == -1) {
> fatalError(R("BBadProp", props[i]));
> + }
>
> String key = props[i].substring(0, equals);
> String value = props[i].substring(equals + 1, props[i].length());
> @@ -325,8 +349,9 @@
> for (int i = 0; i < params.length; i++) {
> // allows empty param, not sure about validity of that.
> int equals = params[i].indexOf("=");
> - if (equals == -1)
> + if (equals == -1) {
> fatalError(R("BBadParam", params[i]));
> + }
>
> String name = params[i].substring(0, equals);
> String value = params[i].substring(equals + 1, params[i].length());
> @@ -381,10 +406,11 @@
> private static String getOption(String option) {
> String result[] = getOptions(option);
>
> - if (result.length == 0)
> + if (result.length == 0) {
> return null;
> - else
> + } else {
> return result[0];
> + }
> }
>
> /**
> @@ -398,14 +424,16 @@
>
> for (int i = 0; i < args.length; i++) {
> if (option.equals(args[i])) {
> - if (-1 == doubleArgs.indexOf(option))
> + if (-1 == doubleArgs.indexOf(option)) {
> result.add(option);
> - else if (i + 1 < args.length)
> + } else if (i + 1 < args.length) {
> result.add(args[i + 1]);
> + }
> }
>
> - if (args[i].startsWith("-") && -1 != doubleArgs.indexOf(args[i]))
> + if (args[i].startsWith("-") && -1 != doubleArgs.indexOf(args[i])) {
> i++;
> + }
> }
>
> return result.toArray(new String[result.size()]);
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list