GNOME Bugzilla – Bug 770617
Ensure Geary always displays an app menu
Last modified: 2016-11-29 02:21:08 UTC
The fix for Bug 759980 removed the custom gear menu in favour of letting GTK+ display the app menu on DE shells that do not natively support app menus. However DEs and/or users can modify the default value for Gtk.Settings::gtk-decoration-layout to not include "menu", and in these cases no app menu will be displayed. Geary needs to check whether the GTK setting contains "menu", and if not manually add it using HeaderBar::set_decoration_layout().
Toolkit bug reported in GTK+: Bug 770619
Fixed here: https://github.com/GNOME/geary/pull/13 I was able to reproduce this bug with settings: gsettings set org.gnome.settings-daemon.plugins.xsettings overrides "{'Gtk/ShellShowsAppMenu': <0>, 'Gtk/DecorationLayout': <':minimize,maximize,close'>}" I am not sure what is the policy. Should I also attach patch here?
Ooops... I missed proposed solution to add it in HeaderBar. I will change it later this week.
Hi Kacper, thanks looking into this! It would be good if you could post patches here using `git --format-patch` so they can be reviewed. I actually don't mind your approach here, if we update the GSetting in Geary does that not cause GTK to automatically put an app menu on the Headerbar?
Created attachment 337456 [details] [review] Fixes the problem of not showing menu in certain configurations of decoration-layout and desktop environment Tested on Gnome on Ubuntu 16.04 LTS by applying following settings: With menu gsettings set org.gnome.settings-daemon.plugins.xsettings overrides "{'Gtk/ShellShowsAppMenu': <0>, 'Gtk/DecorationLayout': <'menu:minimize,maximize,close'>}" Without menu: gsettings set org.gnome.settings-daemon.plugins.xsettings overrides "{'Gtk/ShellShowsAppMenu': <0>, 'Gtk/DecorationLayout': <':minimize,maximize,close'>}" Empty decoration layout: gsettings set org.gnome.settings-daemon.plugins.xsettings overrides "{'Gtk/ShellShowsAppMenu': <0>, 'Gtk/DecorationLayout': <''>}" With stuff at the left gsettings set org.gnome.settings-daemon.plugins.xsettings overrides "{'Gtk/ShellShowsAppMenu': <0>, 'Gtk/DecorationLayout': <'minimize,maximize,close:'>}" Without separator gsettings set org.gnome.settings-daemon.plugins.xsettings overrides "{'Gtk/ShellShowsAppMenu': <0>, 'Gtk/DecorationLayout': <'minimize,maximize,close'>}"
@Michel - it works as expected. After the fix I could see app menu in header bar in all mentioned settings. After enabling app menu in screen top bar ('Gtk/ShellShowsAppMenu': <1>'), menu was displayed only in top screen bar and was not displayed in header bar.
Review of attachment 337456 [details] [review]: Thanks for the patch. This should be good to commit if you can address the minor issues below. ::: src/client/application/geary-controller.vala @@ +176,3 @@ + // See https://bugzilla.gnome.org/show_bug.cgi?id=770617 + Gtk.Settings settings = Gtk.Settings.get_default(); + assert(settings != null); Since get_default() can return null, the type of the settings var should be marked as nullable (i.e. have a "?" at the end). Also since if the assert fails it will cause Geary to crash, so better to put the following code in an if block protected by a null check. @@ +179,3 @@ + + string decoration_layout = settings.gtk_decoration_layout; + assert(decoration_layout != null); Same with this assert. Since neither the property is nullable and the docs don't say it can be null, here it's probably safe to simply assume this will never be null.
Created attachment 337757 [details] [review] Fixes the problem of not showing menu in certain configurations of decoration-layout and desktop environment Thank you for your review. I made it more robust. Application won't crash now when: - Gtk.Settings.get_default() is null - Gtk.Settings.get_default().gtk_decoration_layout is null Let me know if you have any more comments on how I could improve it.
Review of attachment 337757 [details] [review]: Okay, looks good. Committed to master as 67cf38a. I'll ask for some wider testing on the mailing list. Thanks!
I'm running Geary from git master on Ubuntu 16.10. - If I set Ubuntu's option to show menus in title bars, there is still no app menu available in Geary even with this latest change. - Ctrl+Q does work for quitting Geary.
Hmm, can you fire up GTK Inspector again and try a few things out when built with 67cf38a? - What's the value of Geary's GtkSettings gtk-decoration-layout and shell-shows-app-menu properties now with this patch? - If you double-click on the value for gtk-decoration-layout, is the source "XSettings" or "Application"? - If "menu" is still missing from it, does manually adding it to the value make the menu appear in the header bar? E.g. "menu:minimize,..."
On Ubuntu 16.10 running geary master, with Ubuntu's option set to display menus in title bars: gtk-decoration-layout: "menu,close,minimize,maximize:" gtk-shell-shows-app-menu: TRUE gtk-decoration-layout source is Application
Okay, so despite Unity being set to not display the app-menu, it doesn't seem to be actually telling apps that is the case, so they can't decide to show it themselves. I don't suppose the value of gtk-shell-shows-app-menu turns to false if you log out and in again after setting Unity to display menus in title bars? Also, if you adjust the value to false manually using the Inspector, does the app-menu finally appear? If so, I think this is another Unity-specific workaround we'll need to add in 770618.
> I don't suppose the value of gtk-shell-shows-app-menu turns to false if you log out and in again after setting Unity to display menus in title bars? It does not. > Also, if you adjust the value to false manually using the Inspector, does the app-menu finally appear? It does, though it looks very unnatural: I'll attach a screenshot. This is nothing that an Ubuntu user would expect or want. I think that on Unity this bug is fundamentally related to bug 772546 (bring back Unity-specific workarounds). I haven't looked into this at the code level, but I suspect that Unity *really* doesn't want an application to display the app menu itself. Even if the user selects the option to display menus in titlebars, it's still Unity that will display the menu there - that's why gtk-shell-shows-app-menu is true. The fundamental problem is that Geary currently has no title bar, so there's no place for Unity to display the app menu. At least that's my suspicion.
Created attachment 338270 [details] screenshot: app menu on Unity with gtk-shell-shows-app-menu forced to FALSE
Adam, any change with the fix for Bug 772546 landed? Trying that out under Unity 16.10 with a fresh installation, I see the same GtkSettings as you. With the Unity "Show Menus..." pref set to "In the Menu Bar", the app menu appears in the desktop menu bar (the bar along the top of the display) regardless of whether the Geary window is maximised. With menus set to "In the window's title bar", when maximised the app menu still appears in the desktop menu bar, but when windowed the menu appears in the window title bar. This all seems to be by design for Unity, right? Note now however I can't force GTK+ to display the app menu in the headerbar any more by setting gtk-shell-shows-app-menu to False. I guess GTK+ only does that if the headerbar used for CSD and/or if no title bar is shown? Also, Ctrl+Q works fine, regardless of the setting for Show Menus.
Yes - the behavior that you described seems consistent with other applications on the Ubuntu desktop, and all seems right to me. As far as I can tell we don't need to make any more changes in this arena.
Cool, thanks for the feedback. I'll close this for now then, obviously reopen it if something related pops up.