[rfc][icedtea-web] menu support
Jiri Vanek
jvanek at redhat.com
Thu Nov 27 14:38:40 UTC 2014
On 11/27/2014 03:25 PM, Jie Kang wrote:
>
>
> ----- Original Message -----
>> On 11/14/2014 02:21 PM, Jiri Vanek wrote:
>>> On 11/13/2014 06:17 PM, Jiri Vanek wrote:
>>>> On 11/13/2014 05:59 PM, Jiri Vanek wrote:
>>>>> Hi!
>>>>>
>>>>> This is support for XDG menus.
>>>>>
>>>>> Work can be divided into two bug parts
>>>>> 1) additional features to "install desktop icon" dialogue
>>>>> 2) install the menu item
>>>>>
>>>>> 2:
>>>>> Tested in KDE, Gnome3, mate, xfce. (gnome 2 must-to-do) - see javadoc in
>>>>> XdesktopEntry.java for some
>>>>> interesting differences. But works surprisingly well.
>>>>>
>>>>> 1) our glorious accessdialogue is returning only int. I had to rape it a
>>>>> bit:( The only other
>>>>> workaround is to separate desktops's creation to separate class.
>>>>> COnsidering the many logic in
>>>>> AccessWarningPane I had inclined to those few if lines (anyway there is
>>>>> only small possible
>>>>> workaroound around this "rape 70,40,30,20,0 codes :-/)
>>>>>
>>>>> Happy reading :))
>>>>>
>>>>>
>>>>> J.
>>>>>
>>>>> Pleas enote, that [rfc][icedtea-web] ignore npe in pac,a nd fixing
>>>>> --Xoffliner are also part of this
>>>>> patch. I will remove them in next rounds/before push (As the fixxes were
>>>>> already approved anyway).
>>>>>
>>>>>
>>>>> After this is in/during reviw, I will work on new panel to
>>>>> itweb-settings, which allows removing of
>>>>> icons and menu entries added by javaws.itweb.
>>>>>
>>>>>
>>>>> General thoughs?
>>>>>
>>>>> J.
>>>>
>>>> Jsut realized an NPE - in case of ALWAYS_ASK, the information, and
>>>> getShortcut may be null...
>>>>
>>>>
>>>> J.
>>>>
>>> just adapted to head.. Hei I like this menu +offline support :))
>>
>>
>> Fixed few messages - thay will go to properties at the end anyway. Also I
>> have chnaged default icon
>> from java to javaws. My system dont know javaicon, but knows javaws (wellif
>> you run jnlp you have to
>> installe dbooth java, and javaws, but javaws icon is under itw maintainance)
>>
>> Actually - I have this line in fedora specfile:
>> cp javaws.png $RPM_BUILD_ROOT%{_datadir}/pixmaps
>>
>> Shouldnt our make install do so? (afaik yes)
>
>
>
> Hello,
>
>
> First: Pretty awesome work!!! :D
>
>
>
> User-testing on F20 it seems fine, the shorcuts get created and seem to be coded properly. However, not all applets are able to launched through shortcut so it's kind of iffy there. *shrugs*
>
> When testing: http://jogamp.org/deployment/jogamp-next/jogl-applet-version-napplet.html
>
> I was able to create a shortcut for it, and it ended up being a shortcut to an html file. However, when I launched it, it was not opened in my default browser, instead it was attempted to be opened in javaws :\ I think this might be a bug. If it opened in firefox, it would've worked perfectly.
>
>
>
> When testing: http://www.arbores.ca/PresentFuture.jnlp
>
> I encountered:
>
> java.io.FileNotFoundException: /home/jkang/.local/share/applications/javaws/Present/Future Value Calculator.desktop (No such file or directory)
> at java.io.FileOutputStream.open(Native Method)
> at java.io.FileOutputStream.<init>(FileOutputStream.java:221)
> at java.io.FileOutputStream.<init>(FileOutputStream.java:171)
> at net.sourceforge.jnlp.util.XDesktopEntry.installMenuLauncher(XDesktopEntry.java:247)
> at net.sourceforge.jnlp.util.XDesktopEntry.createDesktopShortcuts(XDesktopEntry.java:232)
> at net.sourceforge.jnlp.runtime.ApplicationInstance.addMenuAndDesktopEntries(ApplicationInstance.java:179)
> at net.sourceforge.jnlp.runtime.ApplicationInstance.initialize(ApplicationInstance.java:144)
> at net.sourceforge.jnlp.Launcher.launchApplication(Launcher.java:531)
> at net.sourceforge.jnlp.Launcher$TgThread.run(Launcher.java:912)
>
> Not entirely sure what happened, but the shortcut didn't get created :(
Those are perfectly known issues to me. On top of this patch I have prety bug patch whih is allowing
ahml applets to be working as menu entries.
So please keep this disfunctionality out of review now.
- basicallky - I'm allowing two and half types of browser-applets shortcuts:
1) - I fpound system browser, and do plain entry llike: firefox html_yuo_are_on - working fine
unless your browser is chrome...
- the search is simlar as "find on path" approach
2) I generate jnlp <applet-desc> file form html - this iw working surprisngly awesome! And all
applets I tested run well without browser!
3) I'm using jnlp_hreff-ed file as target jnlp -this is working surprisngly wrong :(( Some
crucial elements are mostly miissing (eg I need to check if codebase , title and vendor are included
and add them if they are not.
4) in progress - instead of our -jnlp argument, I wont to intorducece -html argument - and so be
able to lunch out of browser applet, bat read from html's applet/object tag. Not sure how sucessfull
this willbe:(
Thoughts??
For nits in commnets:
Thank you for review, I will post updtaed patch tomorrow.
As for unclear location - I really wont to include tab in itweb-settings, which will allow to
manage those (but after html shortcuts are done)
J.
>
>
> There are a few nits for the code:
>
> Biggest one for me is: Please add more logging messages to make it easier to debug later on: e.g. where shortcut is being saved, etc.
>
> Smallest one is: There are a few typos, please try to fix them
>
>
> Actual code comments:
>
> diff -r e57ab037cbd0 netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java
> --- a/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java Fri Nov 14 14:06:06 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java Fri Nov 14 15:16:15 2014 +0100
> [...]
>
> - private int[] VALID_ICON_SIZES = new int[] { 16, 22, 32, 48, 64, 128 };
> + private final int[] VALID_ICON_SIZES = new int[] { 16, 22, 32, 48, 64, 128 };
>
> Please add a comment saying what unit these sizes are (I'm guessing pixels?)
>
>
> + public Reader getContentsAsReader(boolean menu) {
>
> String cacheDir = JNLPRuntime.getConfiguration()
> .getProperty(DeploymentConfiguration.KEY_USER_CACHE_DIR);
> File cacheFile = CacheUtil.getCacheFile(file.getSourceLocation(), null);
>
> Both cacheDir and cacheFile are now unused, please remove these two lines.
>
>
> + private static class IconsCreationDescriptor{
> +
> [...]
> + public boolean isMenu() {
> + return menu;
> + }
> +
> +
> +
> +
>
> These extra spaces should be removed;
>
>
> diff -r e57ab037cbd0 netx/net/sourceforge/jnlp/util/XDesktopEntry.java
> --- a/netx/net/sourceforge/jnlp/util/XDesktopEntry.java Fri Nov 14 14:06:06 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/util/XDesktopEntry.java Fri Nov 14 15:16:15 2014 +0100
> [...]
> + private void cacheIcon() throws IOException {
> [...]
> + String origLocation = location.substring("file:".length());
>
> Spacing here needs to be fixed
>
>
> + private static String findAndVerifyJavawsMenuDir() {
> + File f = PathsAndFiles.MENUS_DIR.getFile();
> + String ff = f.getAbsolutePath();
>
> Please rename String ff to something like 'path' or 'menuDir', something that says what the string is representing;
>
>
> private static String evaluateLinuxVariables(String orig, Map<String, String> variables) {
> Set<Entry<String, String>> env = variables.entrySet();
> - List<Entry<String, String>> envVariables = new ArrayList<Entry<String, String>>(env);
>
> This could be shortened to:
>
> - Set<Entry<String, String>> env = variables.entrySet();
> + List<Entry<String, String>> envVariables = new ArrayList<>(variables.entrySet());
>
> I don't see 'env' being used anywhere else and I don't think the extra line to be specific is useful.
>
>
> diff -r e57ab037cbd0 netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java
> --- a/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java Fri Nov 14 14:06:06 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java Fri Nov 14 15:16:15 2014 +0100
> @@ -141,11 +141,6 @@
> [...]
> + File possibleMenuFile = entry.getLinuxMenuIconFile();
> + //if one of menu or desktop exists, do not bother user
> + boolean exists = false;
>
> If I am reading this right, I think this is a pretty annoying flaw. If a user creates either a menu or desktop shortcut, they can no longer create a shortcut of the other type? That sounds bad to me, can this be reworked?
>
> + boolean trullyTrue = (sd != null && (sd.onDesktop() || sd.getMenu() != null));
>
> Please find a better name for this boolean. What does it even check for?
>
>
> diff -r e57ab037cbd0 netx/net/sourceforge/jnlp/config/PathsAndFiles.java
> --- a/netx/net/sourceforge/jnlp/config/PathsAndFiles.java Fri Nov 14 14:06:06 2014 +0100
> +++ b/netx/net/sourceforge/jnlp/config/PathsAndFiles.java Fri Nov 14 15:16:15 2014 +0100
> @@ -53,6 +53,7 @@
> [...]
> + private static final String DATA_HOME;
>
> I am not sure this is the best name. What is DATA? Atm this name is way too generic, and either needs a comment or renaming.
>
>
>
>
>
> Regards,
>
>>
>>
>>
>> J.
>>
>
More information about the distro-pkg-dev
mailing list