Reviewer needed - fix for Re: Strange behaviour during javaws -about
Jiri Vanek
jvanek at redhat.com
Thu Feb 24 09:20:12 PST 2011
On 02/24/2011 06:04 PM, Dr Andrew John Hughes wrote:
> 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.
>
All the formating chanegs are {} behind if. Their's existence protect
from stupid mistake.
>>> 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?
about.jnlp was laoded from inside from jar. No it is loaded from
directory where netx.jar is placed.
>
>> + * extras/net/sourceforge/jnlp/about/Main.java: removed ivokelater,
>> useless hypertextlistener and runtime which needed special permissions.
>
> Why is it useless?
>
Without permissions, the likns can not be lunched by clicking. Most of
the likns are on second tab with demos, which should be removed from my
point of view... Eh bed words maybe. Runtime is useless without
hyperlinklistener and without invoke later, but it itself needed special
permissions should it have to be removed.
> 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?
>
as explained^^
>> 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.
>
I really hate missing brackets behind if() structure. SO I have added
them. As a cosmetic change it should be another patch.
>> 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?
This extracts full path to netx.jar (and from it) to about.jnlp file.
I do not know different way ho to mine this information.
>
>> } 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())
snip
(hope that I have not overlooked comment)
Regards J.
More information about the distro-pkg-dev
mailing list