GNOME Bugzilla – Bug 720552
Provide a more "native" Mac OS application menu
Last modified: 2014-01-18 04:15:57 UTC
Most (all?) applications on Mac OS have an application menu with more or less the same layout. Let's make sure Gtk applications look the same way.
Created attachment 264319 [details] [review] quartz menu: change sensitivity approach By default, Mac OS scans menus as they are opened, updating the sensitivity of each item in the menu. The current code in gtkapplication-menu-quartz disables this behaviour, preferring to manually control the sensitivity of each item in the menu (when told by the tracker that it has changed internally). Change the way that this works to more closely follow the usual Mac OS regime. This will allow us to construct a typical "application menu" on Mac OS containing the items that are typically found there ("Hide", "Hide Others", "Show All", "Services") and have the OS automatically update their sensitivity.
Created attachment 264320 [details] [review] quartz menu: add a hack for application name Add a private hack to allow the insertion of the name of the application into the label of menu items. If it appears in the label of any menu item, the string "`gtk-private-appname`" will be replaced with the name of the application. We will use this for the "Hide myapp", "Quit myapp" and "About myapp" labels typically found on Mac OS programs.
Created attachment 264321 [details] [review] GtkMenuTracker: add 'special' items Allow the possibility for items to be marked with a special attribute and expose this via GtkTrackerMenuItem. For internal use only. We will use this to implement the special 'Hide', 'Hide Others' and 'Show All' items and the 'Services' submenu in the Mac OS application menu.
Created attachment 264322 [details] [review] quartz menu: add special items Add support for the "Hide app", "Hide Others" and "Show All" special items and for the "Services" submenu.
Created attachment 264323 [details] [review] quartz: add a default application menu When running on quartz, it is no longer expected for applications to provide their own application menu. Instead, they should simply ensure that they provide "app.about", "app.preferences" and "app.quit" actions (which many apps are already doing). A default menu will be shown that looks like the one presented by all other Mac OS applications, containing menu items for the above actions, as well as the typical "Hide app", "Hide Others and "Show All" items and the "Services" submenu. If an application does explicitly set an application menu (via gtk_application_set_app_menu()) then it will be respected, as before.
These patches depend on Will's menu work having landed.
I think this is a step back for portability. App menus were introduced with the promise that they would behave 'right' on OS X. And now you're telling us that we're not supposed to use an app menu on OS X anymore ? And instead there's magical action names ?
They behave correctly in the sense that the content you provide is the content that appears as the application menu. Unfortunately, this clashes with the 'native look and feel'. It's my opinion that we should try to come across the same as other apps on a given platform -- or at least give our application authors the tools by which they can do this without too much trouble. This is all going toward the bigger question of how much support we ought to have for menu customisation on different platforms, of course... which is an area that I think we're bound to disagree on.
Why is the special gtk-private-name hack necessary ? Shouldn't e.g. the gimp just use 'About GIMP' as label ? That string is already among the translations anyway, so I don't really see why you win by this, other than a way to autogenerate this string (with the associated translation issues that brings). And if you want to autogenerate it, why not use g_get_application_name() - then we could talk about having a convention for default app menus that works across platforms.
The problem is that the appname appears in 4 places: - the name of the application menu (which the OS will fill in for us) - "About foo" - "Hide foo" - "Quit foo" If we assume that we don't want to have each application manually create the content of the app menu on Mac OS then we need to have the layout of this menu living in Gtk, which means that we need to have (and translate) those strings ourselves. I want to use the same name as OS X uses, which is why I use the Cocoa way to get the name. This will ensure that "About foo" appears properly directly below the menu named "foo", although we could argue that it's up to app authors to make sure they keep those in sync. I don't consider "Hide `gtk-private-appname`" to any different than "Hide %s", if we put a suitable translator comment in place. From my research, it seems to be on the app to provide translations for these strings. I created a new XCode project which gave me this menu layout hardcoded in my .xib/.nib file, then I switched my system language to French, but my app still showed an English menu because the app didn't have French translation. Kinda lame, but that seems to be the way it works... fwiw, I don't think we want to have a default app menu on other platforms. This is really a Mac thing... If the designers of gnome-shell wanted to talk about a standard layout for all app menus, then we could probably discuss how that could be implemented. Hint: I wouldn't do it the way Apple has...
Do apps never add their own items to the os x menu, but instead just add a separate menus in the menubar ? In that case, should this autogenerated os x menu sit next to the actual app menu ?
> > I don't consider "Hide `gtk-private-appname`" to any different than "Hide %s", > if we put a suitable translator comment in place. Wouldn't that be the more straightforward approach to take then ? Considering that some translators have a really hard time leaving mysterious fixed blobs in messages alone.
I wanted to avoid anything that may _ever_ appear in menu text. One could imagine someone actually wanting a literal '%s' to appear in the menu of (for example) some kind of IDE...
(In reply to comment #11) > Do apps never add their own items to the os x menu, but instead just add a > separate menus in the menubar ? In that case, should this autogenerated os x > menu sit next to the actual app menu ? Some apps do indeed add items to the application menu, but the items are usually quite "special" and I've never seen more than one or two added. Items are _never_ removed. Even in the case that the application doesn't have preferences or an 'about', those items are left there, but greyed out (which is what will happen if you don't have an "app.preferences" action). It will be possible for applications to call get_app_menu() in their startup function and modify the returned GMenuModel. That's how I'd recommend that people who were interested in doing such things proceed when on Mac OS.
I think there is at least three levels of support for 'native OS X' that we can go for here, with increasing level of 'magic': 1) Support the special action names you have here 2) Add a method to return the default OS X menu 3) Automatically install the default menu I'm not entirely decided which level of 'magic' I'm comfortable with.
Created attachment 265745 [details] [review] 0005-quartz-menu-add-a-hack-for-application-name.patch
Review of attachment 265745 [details] [review]: I would still be much more comfortable with "About %s" than with "About 'gtk-application-name'". I don't think supporting a literal %s in the translation for "About foo" or "Hide foo" is a real concern.
Review of attachment 264323 [details] [review]: ::: gtk/gtkapplication-quartz.c @@ +90,3 @@ { GtkApplicationImplQuartz *quartz = (GtkApplicationImplQuartz *) impl; + GSimpleActionGroup *gtkinternal; Is this action group populated anywhere ? ::: gtk/gtkapplication-quartz.ui @@ +21,3 @@ + <section> + <item> + <attribute name='label' translatable='yes'>Hide `gtk-private-appname`</attribute> See previous comments about gtk-private-appname vs translators. But, for translators to get this wrong, first of all they need to see these strings. For that, you need to run gtk-extra-strings, and add the resulting .ui.h file to EXTRA_DIST, POTFILES.in, and git...
Review of attachment 264322 [details] [review]: ::: gtk/gtkapplication-quartz-menu.c @@ +375,3 @@ + if (special && g_str_equal (special, "services-submenu")) + [NSApp setServicesMenu:[self submenu]]; + } Is this really useful for anything ? When I borrowed the wife's Macbook, I couldn't find any app for which this services menu was offering anything useful...
Review of attachment 264321 [details] [review]: Seems ok, but we should probably think about reserving some part of the attribute namespace explicitly.
Review of attachment 264319 [details] [review]: Seems fine to me (to the extent that I can read objective c, anyway...)
Review of attachment 264323 [details] [review]: ::: gtk/gtkapplication-quartz.c @@ +115,3 @@ + * app menu at index 0 in 'combined'. + */ + builder = gtk_builder_new_from_resource ("/org/gtk/libgtk/gtkapplication-quartz.ui"); Looking at this path, I wonder - do we have language anywhere that reserves the '/org/gtk/' slice of the resource space for our own purposes ? If not, we probably should ?
(In reply to comment #17) > I would still be much more comfortable with "About %s" than with "About > 'gtk-application-name'". I don't think supporting a literal %s in the > translation for "About foo" or "Hide foo" is a real concern. The problem with automatically replacing '%s' with the application name is that a %s could end up in a menu either by mistake or (in some weird cases) intentionally -- maybe some weird IDE feature or something... Some might end up thinking that this is an actual feature. (In reply to comment #18) > Is this action group populated anywhere ? Ah... I forgot to finish this. I need to add actual actions here in order for the accels to work. > See previous comments about gtk-private-appname vs translators. But, for > translators to get this wrong, first of all they need to see these strings. For > that, you need to run gtk-extra-strings, and add the resulting .ui.h file to > EXTRA_DIST, POTFILES.in, and git... Right. (In reply to comment #19) > Is this really useful for anything ? When I borrowed the wife's Macbook, I > couldn't find any app for which this services menu was offering anything > useful... I have no idea what it is used for to be honest, but every other app has it so I suspect that we ought to as well. I imagine someone who uses a mac more often might know why this is useful...
Review of attachment 264321 [details] [review]: Let's use the x- system here (ie: x-gtk-...).
Review of attachment 265745 [details] [review]: Okay. I'll redo this patch to use %s but only enable it for items that are marked special. We can mark the about/quit items as nominally special for this purpose.
(In reply to comment #20) > Review of attachment 264321 [details] [review]: > > Seems ok, but we should probably think about reserving some part of the > attribute namespace explicitly. GResource docs say this: """ Note that all resources in the process share the same namespace, so use java-style path prefixes (like in the above example) to avoid conflicts. """ Note as well that we already have quite a lot of resources already under this path.
Created attachment 266331 [details] [review] quartz menu: change sensitivity approach Rebased after menu tracker work landed.
Created attachment 266332 [details] [review] GtkMenuTracker: add 'special' items Now using an x-gtk- prefix.
Created attachment 266333 [details] [review] quartz menu: add special items Rebased.
Created attachment 266334 [details] [review] quartz menu: add a hack for application name Now using "%s" and only enabled for special items.
Created attachment 266335 [details] [review] application: new 'insert action group' private api Add a new private API to GtkApplication akin to gtk_widget_insert_action_group(). We'll use this to insert a few extra actions at the app level with a separate namespace for the special items in the Mac OS application menu.
Created attachment 266336 [details] [review] extract-strings: support GMenu GtkBuilder markup Add support for extracting strings from GMenu markup in GtkBuilder files.
Created attachment 266337 [details] [review] quartz: add a default application menu Actually add the internal actions, fixup the menu model for changes to previous patches (special attribute name, appname substitution), add the .in.h file and put it in POTFILES.in.
Review of attachment 266331 [details] [review]: Tested, does fix the menu item sensitivity.
Review of attachment 266332 [details] [review]: ::: gtk/gtkmenutrackeritem.c @@ +657,3 @@ +gtk_menu_tracker_item_get_special (GtkMenuTrackerItem *self) +{ + const gchar *special = NULL;; There is an extra semi-colon here.
Review of attachment 266333 [details] [review]: ::: gtk/gtkapplication-quartz-menu.c @@ +174,3 @@ + + if (special && g_str_equal (special, "services-submenu")) + [NSApp setServicesMenu:[self submenu]]; It might be easier to represent the services submenu with just a single menu tracker item without submenu, and call [NSApp setServicesMenu:[[[NSMenu alloc] init] autorelease]] instead.
Review of attachment 266334 [details] [review]: ::: gtk/gtkapplication-quartz-menu.c @@ +60,3 @@ gulong trackerItemChangedHandler; GCancellable *cancellable; + BOOL isSpecial; Maybe gboolean instead?
Review of attachment 266336 [details] [review]: ::: gtk/extract-strings.c @@ +49,3 @@ error, G_MARKUP_COLLECT_STRING|G_MARKUP_COLLECT_OPTIONAL, "name", NULL, + G_MARKUP_COLLECT_STRING|G_MARKUP_COLLECT_OPTIONAL, "value", NULL, Why is this needed? Could be explained better in the commit message.
(In reply to comment #38) > It might be easier to represent the services submenu with just a single menu > tracker item without submenu, and call [NSApp setServicesMenu:[[[NSMenu alloc] > init] autorelease]] instead. Agreed. Done. (In reply to comment #39) > Maybe gboolean instead? I made it BOOL because it's on the objc side of the code (it's an ivar) and it's not the return value from any g* function... (In reply to comment #40) > Why is this needed? Could be explained better in the commit message. Ya. Good call. It's totally non-obvious. It's because menu models use <attribute> but <attribute> is also used for a _different_ purpose elsewhere in gtkbuilder markup where is appears with value='' (which is not used in menu models). Will improve the message.
Attachment 266333 [details] pushed as ceeef03 - quartz menu: add special items Attachment 266334 [details] pushed as 5d63ee8 - quartz menu: add a hack for application name
Comment on attachment 266335 [details] [review] application: new 'insert action group' private api Attachment 266335 [details] pushed as 6da7b11 - application: new 'insert action group' private api
Comment on attachment 266336 [details] [review] extract-strings: support GMenu GtkBuilder markup Attachment 266336 [details] pushed as 3f1a413 - extract-strings: support GMenu GtkBuilder markup
Review of attachment 266337 [details] [review]: ::: gtk/Makefile.am @@ +1197,3 @@ $(AM_V_GEN) $(GLIB_COMPILE_RESOURCES) $(srcdir)/gtk.gresource.xml \ --target=$@ --sourcedir=$(srcdir) --c-name _gtk --generate-header --manual-register +gtkresources.c: gtk.gresource.xml gtk-default.css gtk-win32.css gtk-win32-xp.css gtk-win32-base.css gtk-win32-classic.css $(DND_CURSORS) $(COMPOSITE_TEMPLATES) $(template_headers) gtkapplication-quartz.ui gtkapplication-quartz.ui is already added to COMPOSITE_TEMPLATES, so doesn't need to be added here. ::: gtk/gtkapplication-quartz.c @@ +139,3 @@ + gtk_application_add_accelerator (impl->application, "<Primary><Alt>h", "gtkinternal.hide-others", NULL); + gtk_application_add_accelerator (impl->application, "<Primary>h", "gtkinternal.hide", NULL); + gtk_application_add_accelerator (impl->application, "<Primary>q", "app.quit", NULL); These accelerators will be added whether or not the application provides its own app menu. Possibly they shouldn't be added if it does, but not sure. ::: gtk/gtkapplication-quartz.ui @@ +21,3 @@ + <attribute name='label' translatable='yes'>Services</attribute> + <attribute name='x-gtk-private-special'>services-submenu</attribute> + </submenu> Possibly make this an item instead of a submenu, as described in https://bugzilla.gnome.org/show_bug.cgi?id=720552#c38.
(In reply to comment #45) > gtkapplication-quartz.ui is already added to COMPOSITE_TEMPLATES, so doesn't > need to be added here. Good catch. Thanks. > These accelerators will be added whether or not the application provides its > own app menu. Possibly they shouldn't be added if it does, but not sure. We talked this over and person and decided that this is probably the right thing to do -- people will generally expect ⌘H to hide the application on Mac OS and if people are going to try to port their app over there then this is something that they will have to be aware of. ie: if they have another shortcut on <Primary>h then they will just have to fix that. Future work here, though: we need to accept shortcuts like '⌘,' and '⌘Q' even when there are no windows open. This speak to the need for a solution in Gdk, perhaps... Which in turn brings up the point of app lifecycle: closing the app when all windows close is not the correct thing to do on Mac OS.... > ::: gtk/gtkapplication-quartz.ui > @@ +21,3 @@ > + <attribute name='label' translatable='yes'>Services</attribute> > + <attribute name='x-gtk-private-special'>services-submenu</attribute> > + </submenu> > > Possibly make this an item instead of a submenu, as described in > https://bugzilla.gnome.org/show_bug.cgi?id=720552#c38. Fixed.
Attachment 266337 [details] pushed as fdc66af - quartz: add a default application menu