Patch for review: Enabling Netx Desktop Element

[Deepak Bhole]

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

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

Cheers,
Deepak