Created attachment 277182 [details] [review]
Add support for app-menu button in window decorations

We want to synchronize the button layouts of our server side
decorations and GTK+'s client side ones. However each currently
may contain buttons not supported by the other, which makes this
unnecessarily tricky.
So add support for a new "appmenu" button in the layout, to display
the fallback app menu in the decorations.

Created attachment 277183 [details] [review]
Actually implement opening the app menu

The last commit added support for the "appmenu" button in decorations,
but didn't actually implement it. Add a new MetaWindowMenuType parameter
to the show_window_menu () functions and use it to ask the compositor
to display the app menu when the new button is activated.

Created attachment 277184 [details] [review]
Support (fallback) app menu in SSD

We now allow "appmenu" in the button layout to make synchronizing it
with GTK+'s client-side decorations easier, but as some people tweak
their settings to get in-window app menus even when using the shell,
actually pop up the app menu when the button is activated.

Created attachment 277185 [details] [review]
window: Add accessor method for gtk_theme_variant

This is useful information for UI associated with a window, so add
a corresponding method.

Created attachment 277186 [details] [review]
theme: Bring fallback app-menu style closer to GTK+

The fallback app-menu in GTK+'s client side decorations obviously
uses the GTK+ theme rather than the shell one; update the style
of our own fallback app-menu to resemble that style.

Review of attachment 277182 [details] [review]:

::: src/core/prefs.c
@@ +1225,3 @@
+ * and assume that we always show the app menu, not least
+ * because we rely on the compositor implementation to display
+ * the fallback ...

Technically, we could be using another compositor implementation that doesn't manage the app menu, but that's not very likely. We sort of have this problem in the Wayland code as well, so maybe it's worth writing something a little less hacky than this. Meta.set_shows_app_menu(true); ?

::: src/wayland/meta-wayland-surface.c
@@ +785,3 @@
     return;
 
+  meta_window_show_menu (surface->window, META_WINDOW_MENU_WM, x, y);

Bad rebase? This symbol isn't changed, and I can't find a definition for the constant.

Review of attachment 277183 [details] [review]:

OK.

Review of attachment 277184 [details] [review]:

::: js/ui/windowMenu.js
@@ +136,3 @@
+
+        this.connect('activate', function() {
+            window.check_alive(global.get_current_time());

I can't help but wonder if we shouldn't just do this whenever we pop up a window below.

@@ +146,3 @@
+});
+
+

Extra whitespace.

::: src/shell-wm.c
@@ +143,3 @@
                   0, NULL, NULL, NULL,
+                  G_TYPE_NONE, 4,
+                  META_TYPE_WINDOW, G_TYPE_INT, G_TYPE_INT, G_TYPE_INT);

Not going to use the enum types? :)

Review of attachment 277185 [details] [review]:

OK.

Review of attachment 277186 [details] [review]:

Should we also do this for the window menu, so it doesn't look too out of place? I guess it's up to the designers, and it's not like it's hard to change once we get there.

(In reply to comment #6)
> ::: src/core/prefs.c
> @@ +1225,3 @@
> maybe it's worth writing something a little less hacky than this. 
> Meta.set_shows_app_menu(true); ?

Not sure how's that really better - the non-hacky way would be to properly follow the XSetting, e.g. always show the fallback when Gtk/ShellShowsAppMenu is false. I have zero intention to actually add a second app menu implementation for the fallback case in mutter, so all the function could do is
 (1) make sure the button is correctly shown when the shell
     does not actually show the app menu, even if though the
     button would not actually work
 (2) adjust the condition for showing the button to something like:
     shellShowsAppMenu == TRUE && overrideShellShowsAppMenu == FALSE
     We'd avoid showing a non-functional button, but the code is just
     plain confusing IMHO.
I'll attach a different approach to (2) which would also work for the window menu.
     

> ::: src/wayland/meta-wayland-surface.c
> Bad rebase?

Yes, fixed locally.



(In reply to comment #8)
> Review of attachment 277184 [details] [review]:
> I can't help but wonder if we shouldn't just do this whenever we pop up a
> window below.

You mean doing that in WindowMenuManager? Sure, could move that check there (though technically everything except "close" still works on frozen windows in X11)


> ::: src/shell-wm.c
> +                  META_TYPE_WINDOW, G_TYPE_INT, G_TYPE_INT, G_TYPE_INT);
> 
> Not going to use the enum types? :)

We are not installing the generated header, not sure it's worth doing for just one signal definition.

Created attachment 277246 [details] [review]
frame: Only allow MENU button when supported by the compositor

The window menu implementation has been moved entirely into the
compositor, so hide the MENU button when the compositor does not
implement corresponding functionality rather than showing a
non-functional button in the decorations.

Review of attachment 277246 [details] [review]:

Since we've killed off lots of other features, maybe we should just say that mutter really only supports the gnome-shell plugin realistically?

You decide if you want to push this one or not.

(In reply to comment #11)
> You mean doing that in WindowMenuManager? Sure, could move that check there
> (though technically everything except "close" still works on frozen windows in
> X11)

Resize?

> We are not installing the generated header, not sure it's worth doing for just
> one signal definition.

Yeah, fine by me then.

Comment on attachment 277246 [details] [review]
frame: Only allow MENU button when supported by the compositor

(In reply to comment #13)
> Since we've killed off lots of other features, maybe we should just say that
> mutter really only supports the gnome-shell plugin realistically?

I'd put this as "someone doing a different plugin can contribute patches when stuff doesn't work out for them", but yeah, not caring much about non-gnome-shell cases at this point ...

Attachment 277182 [details] pushed as c2ea650 - Add support for app-menu button in window decorations
Attachment 277183 [details] pushed as 31db32e - Actually implement opening the app menu
Attachment 277185 [details] pushed as 0a9187a - window: Add accessor method for gtk_theme_variant

Attachment 277184 [details] pushed as 8811ba2 - Support (fallback) app menu in SSD
Attachment 277186 [details] pushed as a72a24e - theme: Bring fallback app-menu style closer to GTK+