GNOME Bugzilla – Bug 688972
gnome-menus tries to use "${XDG_MENU_PREFIX}applications-merged" instead of "applications-merged"
Last modified: 2012-12-06 01:43:45 UTC
The desktop menu spec[1] states that $XDG_CONFIG_DIRS/menus/applications-merged/ need to be used as the default merge directories, no matter whether or not XDG_MENU_PREFIX is set. However, when XDG_MENU_PREFIX is set, gnome-menus fails to merge "applications-merged", and instead attempts to load the non-existent $XDG_CONFIG_DIRS/menus/${XDG_MENU_PREFIX}applications-merged/ (e.g. "gnome-applications-merged") directory. As a result, on Gentoo, where we use XDG_MENU_PREFIX="gnome-", all Wine applications get shoved into the "Other" category in gnome-shell's application chooser. This is happening because resolve_default_merge_dirs() uses the root layout node's name when determining the names of directories to search, and menu_layout_load() by default sets the root node's name to the filename of the menu file, i.e. in this case to "gnome-applications". Note that menu_layout_load() does have a non_prefixed_basename parameter for this very situation and so allows setting a different root node name. But in the chain of function calls in gmenu-tree.c, information about the menu's basename gets lost, and menu_layout_load() ends up being called with a NULL non_prefixed_basename parameter. [1] http://standards.freedesktop.org/menu-spec/latest/ar01s02.html
the analysis looks correct to me
Created attachment 229792 [details] [review] proposed patch Here's a patch to ensure that menu_layout_load() gets called with non_prefixed_name parameter properly set when either loading a menu tree for "applications.menu" (which is what ordinary menu viewers like gnome-shell do) or for "${XDG_MENU_PREFIX}applications.menu" (menu editors do this to save user's changes correctly). Seems to fix the problem for me; tested with gnome-shell, alacarte, gmenu-simple-editor.
Comment on attachment 229792 [details] [review] proposed patch It looks to me that it'd be easier to save the non-prefixed basename in gmenu_tree_canonicalize_path() (since this is where we consume XDG_MENU_PREFIX) instead of adding a new gmenu_tree_update_non_prefixed_basename() method called from set_property().
Created attachment 229969 [details] [review] proposed patch, v2 (In reply to comment #3) OK, here is an updated patch that sets tree->non_prefixed_basename in gmenu_tree_canonicalize_path()
Review of attachment 229969 [details] [review]: Thanks for the update, see comments below. ::: libmenu/gmenu-tree.c @@ +419,1 @@ + if (xdg_menu_prefix != NULL) I'd keep the 'strcmp (tree->basename, "applications.menu") == 0' check here, to avoid doing the rest if not needed. @@ +424,3 @@ + xdg_menu_prefix); + if (!g_strcmp0 (tree->basename, "applications.menu") || + !g_strcmp0 (tree->basename, prefixed_basename)) FWIW, I actually believe it's wrong to compare the non-prefixed basename with ${XDG_MENU_PREFIX}applications.menu: if someone wants to really directly open that file, then the behavior should be different.
(In reply to comment #5) > FWIW, I actually believe it's wrong to compare the non-prefixed basename with > ${XDG_MENU_PREFIX}applications.menu: if someone wants to really directly open > that file, then the behavior should be different. Just to be clear: the only behavior that we are discussing is whether or not to use "applications-merged" as the default merge directory. The standard implies that this particular behavior should be the same: "Note that a system that uses either gnome-applications.menu or kde-applications.menu depending on the desktop environment in use must still use applications-merged as the default merge directory in both cases."[1] Applications that use gnome-menus set "${XDG_MENU_PREFIX}applications.menu" as the basename not because they want to directly open that file (libmenu handles that part fine!), but because they want to *save* changes to the correct files in ~. For example, see bug #683535. Alacarte calls GMenu.Tree.new(os.environ['XDG_MENU_PREFIX'] + 'applications.menu', ...), but its users still expect it to load custom menu categories from "applications-merged" (which exists) and not from "${XDG_MENU_PREFIX}applications-merged" (which doesn't). Vincent, also, look at your own commit from two years ago [2] :) [1] http://standards.freedesktop.org/menu-spec/latest/ar01s02.html [2] http://git.gnome.org/browse/gnome-menus/commit/?id=2054996af842e6df1726e58d81a569f03e30d75fchrome://itsalltext/locale/gumdrop.png
(In reply to comment #6) > [2] http://git.gnome.org/browse/gnome-menus/commit/?id=2054996af842e6df1726e58d81a569f03e30d75fchrome://itsalltext/locale/gumdrop.png Sorry, that should be http://git.gnome.org/browse/gnome-menus/commit/?id=2054996af842e6df1726e58d81a569f03e30d75f
Comment on attachment 229969 [details] [review] proposed patch, v2 Good point, indeed. Can you just add a comment in the code to remind readers about this? :-) Then it's okay to commit. (I'm assuming you've tested this, etc., obviously)
(In reply to comment #8) Committed to master as http://git.gnome.org/browse/gnome-menus/commit/?id=9d45c5369f4a486d2ba5c3a8c3262c84172b095a with the extra comment in the code.