Patch for review: Enabling Netx Desktop Element
Deepak Bhole
dbhole at redhat.com
Thu Dec 3 10:11:15 PST 2009
* Man Wong <mwong at redhat.com> [2009-12-03 13:00]:
> >
> > > * 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
>
>
> Updated the patch to add the quotation marks back in.
>
> Any other issues?
>
Looks good. OK to commit!
Cheers,
Deepak
> 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;
>
> diff -r d9377bd6e521 rt/net/sourceforge/jnlp/runtime/ApplicationInstance.java
> --- a/rt/net/sourceforge/jnlp/runtime/ApplicationInstance.java Fri Nov 27 11:17:31 2009 -0500
> +++ b/rt/net/sourceforge/jnlp/runtime/ApplicationInstance.java Tue Dec 01 15:05:42 2009 -0500
> @@ -109,24 +109,12 @@
> */
> public void initialize() {
> installEnvironment();
> -
> - /*
> - * FIXME: Disable creating desktop entries for now
> - *
> - * there are some major issues we need to work out before we can enable them
> - * 1. Playing nice with the altnatives system
> - * - use the system preferred jdk (/usr/bin/javaws)
> - * - dont assume what jdk javaws corresponds to
> - * - make sure our shortcuts work with the sun jdk and vice versa
> - * (may not be possible since sun's javaws creates a launcher that
> - * links to /usr/java/${ver}/bin/javaws)
> - * - we should use the same options and arguments as sun's javaws
> - * 2. Make shortcuts work offline
> - * - make the cache updates and replacements work properly
> - * - shortcuts should use the cache
> - *
> - * addMenuAndDesktopEntries();
> - */
> +
> + //Fixme: -Should check whether a desktop entry already exists for
> + // for this jnlp file, and do nothing if it exists.
> + // -If no href is specified in the jnlp tag, it should
> + // default to using the one passed in as an argument.
> + addMenuAndDesktopEntries();
> }
>
> /**
> @@ -135,14 +123,15 @@
>
> private void addMenuAndDesktopEntries() {
> XDesktopEntry entry = new XDesktopEntry(file);
> + ShortcutDesc sd = file.getInformation().getShortcut();
>
> - if (file.getInformation().getShortcut().onDesktop()) {
> + if (sd != null && sd.onDesktop()) {
> if (ServiceUtil.checkAccess(this, AccessType.CREATE_DESTKOP_SHORTCUT)) {
> entry.createDesktopShortcut();
> }
> }
>
> - if (file.getInformation().getShortcut().getMenu() != null) {
> + if (sd != null && sd.getMenu() != null) {
> /*
> * Sun's WebStart implementation doesnt seem to do anything under GNOME
> */
> diff -r d9377bd6e521 rt/net/sourceforge/jnlp/runtime/Boot.java
> --- a/rt/net/sourceforge/jnlp/runtime/Boot.java Fri Nov 27 11:17:31 2009 -0500
> +++ b/rt/net/sourceforge/jnlp/runtime/Boot.java Tue Dec 01 15:05:42 2009 -0500
> @@ -278,6 +278,11 @@
>
> JNLPFile file = new JNLPFile(url, strict);
>
> + // Launches the jnlp file where this file originated.
> + if (file.getSourceLocation() != null) {
> + file = new JNLPFile(file.getSourceLocation(), strict);
> + }
> +
> // add in extra params from command line
> addProperties(file);
>
> 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";
>
> return new StringReader(fileContents);
>
More information about the distro-pkg-dev
mailing list