After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 720552 - Provide a more "native" Mac OS application menu
Provide a more "native" Mac OS application menu
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Class: GtkApplication
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on: 710351 720550
Blocks:
 
 
Reported: 2013-12-16 18:59 UTC by Allison Karlitskaya (desrt)
Modified: 2014-01-18 04:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
quartz menu: change sensitivity approach (3.94 KB, patch)
2013-12-16 18:59 UTC, Allison Karlitskaya (desrt)
reviewed Details | Review
quartz menu: add a hack for application name (1.45 KB, patch)
2013-12-16 18:59 UTC, Allison Karlitskaya (desrt)
none Details | Review
GtkMenuTracker: add 'special' items (2.05 KB, patch)
2013-12-16 18:59 UTC, Allison Karlitskaya (desrt)
reviewed Details | Review
quartz menu: add special items (2.21 KB, patch)
2013-12-16 18:59 UTC, Allison Karlitskaya (desrt)
reviewed Details | Review
quartz: add a default application menu (6.70 KB, patch)
2013-12-16 18:59 UTC, Allison Karlitskaya (desrt)
reviewed Details | Review
0005-quartz-menu-add-a-hack-for-application-name.patch (1.79 KB, patch)
2014-01-08 19:30 UTC, fakey
none Details | Review
quartz menu: change sensitivity approach (2.88 KB, patch)
2014-01-15 06:28 UTC, Allison Karlitskaya (desrt)
committed Details | Review
GtkMenuTracker: add 'special' items (2.05 KB, patch)
2014-01-15 06:29 UTC, Allison Karlitskaya (desrt)
committed Details | Review
quartz menu: add special items (2.11 KB, patch)
2014-01-15 06:29 UTC, Allison Karlitskaya (desrt)
committed Details | Review
quartz menu: add a hack for application name (2.33 KB, patch)
2014-01-15 06:29 UTC, Allison Karlitskaya (desrt)
committed Details | Review
application: new 'insert action group' private api (2.19 KB, patch)
2014-01-15 06:30 UTC, Allison Karlitskaya (desrt)
committed Details | Review
extract-strings: support GMenu GtkBuilder markup (1.50 KB, patch)
2014-01-15 06:36 UTC, Allison Karlitskaya (desrt)
committed Details | Review
quartz: add a default application menu (9.84 KB, patch)
2014-01-15 06:38 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2013-12-16 18:59:17 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.
Comment 1 Allison Karlitskaya (desrt) 2013-12-16 18:59:18 UTC
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.
Comment 2 Allison Karlitskaya (desrt) 2013-12-16 18:59:21 UTC
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.
Comment 3 Allison Karlitskaya (desrt) 2013-12-16 18:59:24 UTC
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.
Comment 4 Allison Karlitskaya (desrt) 2013-12-16 18:59:26 UTC
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.
Comment 5 Allison Karlitskaya (desrt) 2013-12-16 18:59:29 UTC
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.
Comment 6 Allison Karlitskaya (desrt) 2013-12-16 19:00:28 UTC
These patches depend on Will's menu work having landed.
Comment 7 Matthias Clasen 2013-12-17 19:08:32 UTC
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 ?
Comment 8 Allison Karlitskaya (desrt) 2013-12-17 19:58:52 UTC
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.
Comment 9 Matthias Clasen 2013-12-17 20:18:46 UTC
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.
Comment 10 Allison Karlitskaya (desrt) 2013-12-17 20:25:07 UTC
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...
Comment 11 Matthias Clasen 2013-12-17 21:57:58 UTC
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 ?
Comment 12 Matthias Clasen 2013-12-17 22:02:45 UTC
> 
> 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.
Comment 13 Allison Karlitskaya (desrt) 2013-12-17 22:09:41 UTC
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...
Comment 14 Allison Karlitskaya (desrt) 2013-12-17 22:11:39 UTC
(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.
Comment 15 Matthias Clasen 2013-12-19 22:15:32 UTC
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.
Comment 16 fakey 2014-01-08 19:30:58 UTC
Created attachment 265745 [details] [review]
0005-quartz-menu-add-a-hack-for-application-name.patch
Comment 17 Matthias Clasen 2014-01-14 21:40:27 UTC
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.
Comment 18 Matthias Clasen 2014-01-14 22:18:24 UTC
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...
Comment 19 Matthias Clasen 2014-01-14 22:26:41 UTC
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...
Comment 20 Matthias Clasen 2014-01-14 22:41:08 UTC
Review of attachment 264321 [details] [review]:

Seems ok, but we should probably think about reserving some part of the attribute namespace explicitly.
Comment 21 Matthias Clasen 2014-01-14 22:42:12 UTC
Review of attachment 264319 [details] [review]:

Seems fine to me (to the extent that I can read objective c, anyway...)
Comment 22 Matthias Clasen 2014-01-14 22:44:46 UTC
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 ?
Comment 23 Allison Karlitskaya (desrt) 2014-01-15 02:33:16 UTC
(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...
Comment 24 Allison Karlitskaya (desrt) 2014-01-15 05:19:33 UTC
Review of attachment 264321 [details] [review]:

Let's use the x- system here  (ie: x-gtk-...).
Comment 25 Allison Karlitskaya (desrt) 2014-01-15 05:20:36 UTC
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.
Comment 26 Allison Karlitskaya (desrt) 2014-01-15 05:44:49 UTC
(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.
Comment 27 Allison Karlitskaya (desrt) 2014-01-15 06:28:57 UTC
Created attachment 266331 [details] [review]
quartz menu: change sensitivity approach

Rebased after menu tracker work landed.
Comment 28 Allison Karlitskaya (desrt) 2014-01-15 06:29:18 UTC
Created attachment 266332 [details] [review]
GtkMenuTracker: add 'special' items

Now using an x-gtk- prefix.
Comment 29 Allison Karlitskaya (desrt) 2014-01-15 06:29:28 UTC
Created attachment 266333 [details] [review]
quartz menu: add special items

Rebased.
Comment 30 Allison Karlitskaya (desrt) 2014-01-15 06:29:57 UTC
Created attachment 266334 [details] [review]
quartz menu: add a hack for application name

Now using "%s" and only enabled for special items.
Comment 31 Allison Karlitskaya (desrt) 2014-01-15 06:30:04 UTC
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.
Comment 32 Allison Karlitskaya (desrt) 2014-01-15 06:36:38 UTC
Created attachment 266336 [details] [review]
extract-strings: support GMenu GtkBuilder markup

Add support for extracting strings from GMenu markup in GtkBuilder
files.
Comment 33 Allison Karlitskaya (desrt) 2014-01-15 06:38:26 UTC
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.
Comment 34 fakey 2014-01-18 01:09:21 UTC
Review of attachment 266331 [details] [review]:

Tested, does fix the menu item sensitivity.
Comment 35 fakey 2014-01-18 01:09:36 UTC
Review of attachment 266331 [details] [review]:

Tested, does fix the menu item sensitivity.
Comment 36 fakey 2014-01-18 01:09:36 UTC
Review of attachment 266331 [details] [review]:

Tested, does fix the menu item sensitivity.
Comment 37 fakey 2014-01-18 01:22:05 UTC
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.
Comment 38 fakey 2014-01-18 02:48:20 UTC
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.
Comment 39 fakey 2014-01-18 02:52:48 UTC
Review of attachment 266334 [details] [review]:

::: gtk/gtkapplication-quartz-menu.c
@@ +60,3 @@
   gulong trackerItemChangedHandler;
   GCancellable *cancellable;
+  BOOL isSpecial;

Maybe gboolean instead?
Comment 40 fakey 2014-01-18 03:16:04 UTC
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.
Comment 41 Allison Karlitskaya (desrt) 2014-01-18 03:31:01 UTC
(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.
Comment 42 Allison Karlitskaya (desrt) 2014-01-18 03:40:27 UTC
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 43 Allison Karlitskaya (desrt) 2014-01-18 03:41:29 UTC
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 44 Allison Karlitskaya (desrt) 2014-01-18 03:45:17 UTC
Comment on attachment 266336 [details] [review]
extract-strings: support GMenu GtkBuilder markup

Attachment 266336 [details] pushed as 3f1a413 - extract-strings: support GMenu GtkBuilder markup
Comment 45 fakey 2014-01-18 03:57:14 UTC
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.
Comment 46 Allison Karlitskaya (desrt) 2014-01-18 04:05:55 UTC
(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.
Comment 47 Allison Karlitskaya (desrt) 2014-01-18 04:15:54 UTC
Attachment 266337 [details] pushed as fdc66af - quartz: add a default application menu