Patch for review: Enabling Netx Desktop Element

[Man Wong]

----- "Deepak Bhole" <dbhole at redhat.com> wrote:

> * Man Wong <mwong at redhat.com> [2009-12-01 17:15]:
> > This patch fixes the 2 problems (shortcut not using system preferred
> javaws and jnlp file in cache) from the previous fixme. The fix allows
> netx desktop element to be enabled. Furthermore, shortcut tag is now
> no longer required as in the jnlp developers guide. Lastly, it now
> checks for updates and updates the file in the cache accordingly.    
> > 
> > Changelog:
> > 2009-12-01  Man Lung Wong <mwong at redhat.com>
> >      
> >          * rt/net/sourceforge/jnlp/JNLPFile.java
> >          (JNLPFILE): Download the jnlp file from the webspace it
> originated, if    
> >          it exists.
> >          * rt/net/sourceforge/jnlp/runtime/ApplicationInstance.java
> >          (initialize): Enable desktop element and revise the list of
> fixme.
> >          (addMenuAndDesktopEntries): No longer errors out when
> shortcut tag not    
> >          specified.
> >          * rt/net/sourceforge/jnlp/runtime/Boot.java
> >          (getFile): Launches the original jnlp file (i.e. if the
> file was
> >          downloaded from http://icedtea.classpath.org, then it will
> launch the
> >          one from http://icedtea.classpath.org).
> >          * rt/net/sourceforge/jnlp/util/XDesktopEntry.java
> >          (getContentsAsReader): Shortcut uses jnlp file in cache and
> launches 
> >          with system preferred javaws.
> > 
> > Any comments?
> > 
> 
> Please see comments below:
> 
> > Thanks,
> > Man Lung Wong
> > diff -r d9377bd6e521 rt/net/sourceforge/jnlp/JNLPFile.java
> > --- a/rt/net/sourceforge/jnlp/JNLPFile.java	Fri Nov 27 11:17:31 2009
> -0500
> > +++ b/rt/net/sourceforge/jnlp/JNLPFile.java	Tue Dec 01 15:05:42 2009
> -0500
> > @@ -173,6 +173,14 @@
> >      public JNLPFile(URL location, Version version, boolean strict,
> UpdatePolicy policy) throws IOException, ParseException {
> >          Node root = Parser.getRootNode(openURL(location, version,
> policy));
> >          parse(root, strict, location);
> > +        	
> > +        //Downloads the original jnlp file into the cache if
> possible
> > +        //(i.e. If the jnlp file being launched exist locally, but
> it 
> > +        //originated from a website, then download the one from the
> website
> > +        //into the cache).  		
> > +        if (sourceLocation != null && location.getProtocol() ==
> "file") {
> > +            openURL(sourceLocation, version, policy);
> > +        }
> >  
> >          this.fileLocation = location;
> >          
> 
> What is the difference between sourceLocation and location? In the if
> condition, one is being checked for notnull, and another is being
> checked for protocol. Is this expected?
>

It is expected, because sourceLocation != null checks if the jnlp file points to another file (which is the original file), while location.getProtocol() == "file" checks if the file being passed in as an argument is a local file (if it is not a local meaning it is from a webspace then it would have been downloaded into the cache by the previous openURL call).
 
> <snip>
> 
> > diff -r d9377bd6e521
> rt/net/sourceforge/jnlp/util/XDesktopEntry.java
> > --- a/rt/net/sourceforge/jnlp/util/XDesktopEntry.java	Fri Nov 27
> 11:17:31 2009 -0500
> > +++ b/rt/net/sourceforge/jnlp/util/XDesktopEntry.java	Tue Dec 01
> 15:05:42 2009 -0500
> > @@ -73,6 +73,7 @@
> >  
> >          String pathToJavaws = System.getProperty("java.home") +
> File.separator + "bin"
> >                  + File.separator + "javaws";
> > +        File cacheFile =
> CacheUtil.urlToPath(file.getSourceLocation(), "cache");
> >  
> >          String fileContents = "[Desktop Entry]\n";
> >          fileContents += "Version=1.0\n";
> > @@ -89,7 +90,9 @@
> >          if (file.getInformation().getVendor() != null) {
> >              fileContents += "Vendor=" +
> file.getInformation().getVendor() + "\n";
> >          }
> > -        fileContents += "Exec=" + pathToJavaws + " \"" +
> file.getSourceLocation() + "\"\n";
> > +        
> > +        //Shortcut executes the jnlp from cache and system
> preferred java..
> > +        fileContents += "Exec=" + "javaws" + " " +
> cacheFile.getAbsolutePath() + "\n";
> >  
> 
> Are you sure the extra quotes (\"'s) should be removed? What if the
> file
> name has spaces, would it still work?
> 

Good catch! Even though, file names with a space cannot be launched. However, it is a problem that should/will be addressed.

> Cheers,
> Deepak

Thanks,

Man Lung Wong