GMenu-DBusMenu-Proxy
ClosedPublic

Authored by broulik on Feb 12 2018, 2:12 PM.

Details

Reviewers
mart
davidedmundson
Group Reviewers
Plasma
Summary

This application finds windows using GTK GMenu DBus interfaces [1] and forwards them through DBusMenu. This way no adjustments in Plasma are needed.

BUG: 375976
FIXED-IN: 5.13.0

[1] https://wiki.gnome.org/Projects/GLib/GApplication/DBusAPI#org.gtk.Menus

Test Plan

Branch broulik/gmenu-dbusmenu-proxy of plasma-workspace

Tested `gedit


Tested LibreOffice 6 with gtk3_kde5 platform plugin, should just work with regular GTK3 plugin; also works with gtk and gtk3 plugins when you have appmenu-gtk-module. Out of the box with gtk3 action IDs are randomly assigned so I cannot map them to any icons, however, with appmenu-gtk-module I can.
LibreOffice vanilla gtk3_kde5

LibreOffice with appmenu-gtk-module

Applications not having a dedicated menu bar will at least show the top panel menu with Settings, About, and Help

Works with Gimp or Inkscape if you have appmenu-gtk-module (there's GTK2 and GTK3 variants) installed and GTK_MODULES=appmenu-gtk-module environment variable set (or unity-gtk-module for pre-Gnome Ubuntu)

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Feb 12 2018, 2:12 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 12 2018, 2:12 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Feb 12 2018, 2:12 PM
broulik updated this revision to Diff 26999.Feb 12 2018, 2:31 PM
broulik edited the summary of this revision. (Show Details)
broulik edited the test plan for this revision. (Show Details)
  • Don't send random arguments to Activate, fixes toggle actions not working
broulik edited the summary of this revision. (Show Details)Feb 12 2018, 2:32 PM
broulik updated this revision to Diff 27077.Feb 13 2018, 4:47 PM
broulik retitled this revision from WIP: GMenu-DBusMenu-Proxy to GMenu-DBusMenu-Proxy.
broulik edited the summary of this revision. (Show Details)
  • Cleanup code and use categorized logging
  • Monitor action changes (e.g. toggled state of checkbox)
  • Monitor menu changes (e.g. menu label changed)
  • Handle when app has no menu on startup but gets one later
  • Disable GTK menu bar when we're running
rilian added a subscriber: rilian.Feb 14 2018, 2:38 AM

Hello, I am Konstantin, developer of vala-panel-appmenu. I have some comments about your application.

MenuModel protocol consitsts for 5 items:

AppMenu - with property _GTK_APPMENU_OBJECT_PATH
MenuBar - with property _GTK_MENUBAR_OBJECT_PATH
It is a menu models, it is how menu should drawn on screen.
One can be missing, and then incomplete menu should render:
a) If AppMenu is missing, you will miss menu entry with application name (vala-panel-appmenu renders stub in place of missing AppMenu)
b) If MenuBar is missing, you will miss all menu entries except entry with application name.
c) If both are missing, you will not see a menu (Protocol is incorrect)

And 3 providers of actions. Actions is required to get menu react on user changes.Providers:

Application (_GTK_APPLICATION_OBJECT_PATH, prefix app) - it is actions from all application (not bound to a particular window)
Window (_GTK_WINDOW_OBJECT_PATH, prefix win) - it is actions from current window, as it set by a developer of application
Unity (_UNITY_OBJECT_PATH, prefix unity) - it is non-standard, but widely used action path for set a Unity actions (when window actions is not supported by app developer). It is object path supported by unity-gtk-module and appmenu-gtk-module.
If you implement this, you will get GTK2 menu for free.
If any of this are missing, this menu items should be rendered as disabled. But if menu using actions only from one category - it can be used as a normal menu. Setting this all is not required for functional menu. One will be enough, if menu is using actions only from one group.
ngraham rescinded a token.
ngraham awarded a token.

Hi Konstantin,
thank you very much for your input!

One can be missing, and then incomplete menu should render:

In Plasma we don't typically use this separate "app menu" (although I'd like to do that but as a separate menu), so what I do is I show the menu bar and if that is not there I'll try the app menu. I never show both of them at once to be consistent with KDE apps that only have a menu.

c) If both are missing, you will not see a menu (Protocol is incorrect)

I don't understand. Incorrect on my side? When both menus are missing where should I get the menu from?

Unity (_UNITY_OBJECT_PATH, prefix unity) - it is non-standard, but widely used action path for set a Unity actions (when window actions is not supported by app developer). It is object path supported by unity-gtk-module and appmenu-gtk-module.

I was already wondering what the unity. prefixed actions I saw mentioned in a script I looked at. I just installed unity-gtk2-module and tried with some GTK2 apps but I don't see any new window properties. What else do I need to get GTK2 working? (Also given Unity is basically dead and a Ubuntu-specifica, won't really help us on non-*buntu distributions, right?)

If any of this are missing, this menu items should be rendered as disabled. But if menu using actions only from one category - it can be used as a normal menu. Setting this all is not required for functional menu. One will be enough, if menu is using actions only from one group.

You mean that I don't neccessarily need both application and window actions in case the menu just uses application actions? I'll fix that.

rk added a subscriber: rk.Feb 14 2018, 9:11 AM

Thanks so much)

  1. Unity is dead, but I forked unity-gtk-module, patched it for all distros, fixed some bugs and called appmenu-gtk-module. So, Unity is dead, but unity action prefixes is live.
  2. To use unity-gtk-module or appmenu-gtk-module you need to install it into correct location (where all gtk modules is located) and then add its name (unity-gtk-module or appmenu-gtk-module) to environment variable called GTK_MODULES. If it will not working with appmenu-gtk-module, write me. unity-gtk-module maintainers is not so responsive.
  3. Menu can use only application actions, only window actions and only unity actions. And all this are correct, as well as any combination of it (but not empty combination).
broulik updated this revision to Diff 27142.Feb 14 2018, 11:41 AM
broulik edited the test plan for this revision. (Show Details)
  • Support unity actions (untested)
  • Don't mandate actions to be present, it's perfectly valid for a window to have a menu only consisting of application actions or vice-versa
  • Improve action update logic, signal ItemsPropertiesUpdated when an action changes instead of rebuilding the layout
  • Improve menu update logic, just update the existing items when the same number of items is removed and added

So, Unity is dead, but unity action prefixes is live.

I just added support for that but I don't know what apps use it to test it, if you may..? :)

To use unity-gtk-module or appmenu-gtk-module you need to install it into correct location (where all gtk modules is located) and then add its name (unity-gtk-module or appmenu-gtk-module) to environment variable called GTK_MODULES.

I just did that and now get a full menu in e.g. Pluma. However, most other apps, like Evince, don't get a menu at all anymore. It seems the the appmenu-gtk-module always announces a menu bar which may be empty? :/ My proxy does not fallback to empty menus, it only falls back when the menu property doesn't exist to begin with and I wouldn't really want to make it even more complex as it is.

  1. Yes, menubar may be empty with appmenu-gtk-module or unity-gtk-module, and if I can fix appmenu-gtk-module, unity-gtk-module will not be fixed. So, I think it is preferred to check on your side.
  2. GTK3 applications (file-roller, for example) can use both appmenu and menubar with different items (and different action) . So, I think, hiding appmenu is bad, because user may want this menu entries.

O, some another note: You can generate menubar from appmenu.
For example, Nautilus:
It have 4 sections

  1. New Window
  2. Sidebar
  3. Preferences
  4. Keyboard Shortcuts Help About Quit.

You can add New Window and Quit to File menu, Sidebar to View, Preferences and Keyboard shortcuts to Tools (or Edit), and Help and About to help.

What about FIXME unity, what are you mean? I can fix appmenu-gtk-module for you.

Thanks for your input!

Yes, menubar may be empty

Okay. Problem is that for example LibreOffice doesn't have a menu right away, so I can't realy tell "no menu because it's still loading" or "no menu because it doesn't have one" and then fallback to app menu. I could perhaps check if the app has an appmenu at all before trying to fallback but not really fond of adding even more complexity to it.

GTK3 applications (file-roller, for example) can use both appmenu and menubar with different items (and different action)

What kind of different actions? So far I have only had redundancy in the app menu, I'll try to look into this, merging two separate menus into one somehow, also getting the app name for the app menu..

You can add New Window and Quit to File menu, Sidebar to View, Preferences and Keyboard shortcuts to Tools (or Edit), and Help and About to help.

How am I supposed to know which action belongs where?

What about FIXME unity, what are you mean?

That was just for the icon mapping, I can probably remove this, since the actions in unity are just their localized labels plus unity. prefix, there's nothing I can map them to (like I would be able to window.open to document-open icon)

If you need help, I will provide it for you, because for me there is 2 features which should be in KDE for me:

  1. Global Menu (for all protocols)
  2. QGtkStyle (with GTK3 themes)

Okay. Problem is that for example LibreOffice doesn't have a menu right away, so I can't realy tell "no menu because it's still loading" or "no menu because it doesn't have one" and then fallback to app menu. I could perhaps check if the app has an appmenu at all before trying to fallback but not really fond of adding even more complexity to it.

I too because MenuModel can be empty on start, and I cannot differ than user turned menu off or just application do not have a menu?
About searching of appmenu and fallback to it - LibreOffice have both appmenu and menubar, so, we will lose LibreOffice menu.

What kind of different actions? So far I have only had redundancy in the app menu, I'll try to look into this, merging two separate menus into one somehow, also getting the app name for the app menu..

It can be actions (GActions, I mean) than exists only in appmenu, but not in menubar. User may want this.

How am I supposed to know which action belongs where?

But all menuitems have "action" attribute)
Or if you about a QAction (which, I think, should called QMenuItem), this is several ways to do this:

  1. Look for each section, name it by some action-name regex (as you did with icons) and then show it as menubar.
  2. Or just do it with each menuitem, but it is way more complicated. I suggest a section-way.

That was just for the icon mapping, I can probably remove this, since the actions in unity are just their localized labels plus unity. prefix, there's nothing I can map them to (like I would be able to window.open to document-open icon)

I think you do not need mapping, because we have a bunch of this code:

C
static GtkImage *gtk_menu_item_get_nth_image(GtkMenuItem *menu_item, guint index)
{
	UnityGtkSearch search;

	g_return_val_if_fail(GTK_IS_MENU_ITEM(menu_item), NULL);

	search.type   = GTK_TYPE_IMAGE;
	search.index  = index;
	search.object = NULL;

	g_object_get_nth_object(G_OBJECT(menu_item), &search);

	return search.object != NULL ? GTK_IMAGE(search.object) : NULL;
}

static GIcon *gtk_image_get_icon(GtkImage *image)
{
	GIcon *icon = NULL;

	g_return_val_if_fail(GTK_IS_IMAGE(image), NULL);

	switch (gtk_image_get_storage_type(image))
	{
	case GTK_IMAGE_GICON:
	{
		gtk_image_get_gicon(image, &icon, NULL);

		if (icon != NULL)
			g_object_ref(icon);
	}

	break;

	case GTK_IMAGE_ICON_NAME:
	{
		const char *name = NULL;

		gtk_image_get_icon_name(image, &name, NULL);

		if (name != NULL)
			icon = G_ICON(g_themed_icon_new_with_default_fallbacks(name));
	}

	break;

	case GTK_IMAGE_PIXBUF:
	{
		GdkPixbuf *pixbuf = gtk_image_get_pixbuf(image);

		if (pixbuf != NULL)
			icon = g_object_ref(pixbuf);
	}

	break;

	case GTK_IMAGE_ANIMATION:
	{
		GdkPixbufAnimation *animation = gtk_image_get_animation(image);

		if (animation != NULL)
		{
			GdkPixbuf *pixbuf = gdk_pixbuf_animation_get_static_image(animation);

			if (pixbuf != NULL)
				icon = g_object_ref(pixbuf);
		}
	}

	break;

	case GTK_IMAGE_STOCK:
#if GTK_MAJOR_VERSION == 2
	{
		char *stock      = NULL;
		GtkIconSize size = GTK_ICON_SIZE_INVALID;

		gtk_image_get_stock(image, &stock, &size);

		if (stock != NULL)
		{
			GdkPixbuf *pixbuf =
			    gtk_widget_render_icon(GTK_WIDGET(image), stock, size, NULL);

			if (pixbuf != NULL)
				icon = G_ICON(pixbuf);
		}
	}
#elif GTK_MAJOR_VERSION == 3
	{
		GtkStyleContext *context = gtk_widget_get_style_context(GTK_WIDGET(image));

		if (context != NULL)
		{
			char *stock      = NULL;
			GtkIconSize size = GTK_ICON_SIZE_INVALID;

			gtk_image_get_stock(image, &stock, &size);

			if (stock != NULL)
			{
				GtkIconSet *set = gtk_style_context_lookup_icon_set(context, stock);

				if (set != NULL)
				{
					GdkPixbuf *pixbuf =
					    gtk_icon_set_render_icon_pixbuf(set, context, size);

					if (pixbuf != NULL)
						icon = G_ICON(pixbuf);
				}
			}
		}
	}
#endif

	break;

	case GTK_IMAGE_ICON_SET:
#if GTK_MAJOR_VERSION == 2
	{
		GtkIconSet *set  = NULL;
		GtkIconSize size = GTK_ICON_SIZE_INVALID;

		gtk_image_get_icon_set(image, &set, &size);

		if (set != NULL)
		{
			GtkStyle *style            = gtk_widget_get_style(GTK_WIDGET(image));
			GtkTextDirection direction = gtk_widget_get_direction(GTK_WIDGET(image));
			GtkStateType state         = gtk_widget_get_state(GTK_WIDGET(image));
			GdkPixbuf *pixbuf          = gtk_icon_set_render_icon(set,
                                                                     style,
                                                                     direction,
                                                                     state,
                                                                     size,
                                                                     GTK_WIDGET(image),
                                                                     NULL);

			if (pixbuf != NULL)
				icon = G_ICON(pixbuf);
		}
	}
#elif GTK_MAJOR_VERSION == 3
	{
		GtkStyleContext *context = gtk_widget_get_style_context(GTK_WIDGET(image));

		if (context != NULL)
		{
			GtkIconSet *set  = NULL;
			GtkIconSize size = GTK_ICON_SIZE_INVALID;

			gtk_image_get_icon_set(image, &set, &size);

			if (set != NULL)
			{
				GdkPixbuf *pixbuf =
				    gtk_icon_set_render_icon_pixbuf(set, context, size);

				if (pixbuf != NULL)
					icon = G_ICON(pixbuf);
			}
		}
	}
#endif

	break;

#if GTK_MAJOR_VERSION == 2
	case GTK_IMAGE_IMAGE:
	{
		GdkImage *gdk_image = NULL;

		gtk_image_get_image(image, &gdk_image, NULL);

		if (gdk_image != NULL)
		{
			GdkColormap *colourmap = gtk_widget_get_colormap(GTK_WIDGET(image));
			GdkPixbuf *pixbuf      = gdk_pixbuf_get_from_image(NULL,
                                                                      gdk_image,
                                                                      colourmap,
                                                                      0,
                                                                      0,
                                                                      0,
                                                                      0,
                                                                      gdk_image->width,
                                                                      gdk_image->height);

			if (pixbuf != NULL)
				icon = G_ICON(pixbuf);
		}
	}

	break;

	case GTK_IMAGE_PIXMAP:
	{
		GdkPixmap *pixmap = NULL;

		gtk_image_get_pixmap(image, &pixmap, NULL);

		if (pixmap != NULL)
		{
			GdkPixbuf *pixbuf;
			GdkColormap *colourmap;
			gint width  = 0;
			gint height = 0;

			gdk_pixmap_get_size(pixmap, &width, &height);
			colourmap = gtk_widget_get_colormap(GTK_WIDGET(image));
			pixbuf    = gdk_pixbuf_get_from_drawable(NULL,
                                                              pixmap,
                                                              colourmap,
                                                              0,
                                                              0,
                                                              0,
                                                              0,
                                                              width,
                                                              height);

			if (pixbuf != NULL)
				icon = G_ICON(pixbuf);
		}
	}

	break;
#endif

	default:
		break;
	}

	return icon;
}

If any icon can be found, appmenu-gtk-module will export this.
And I will temporary disable exporting an empty menubar to MenuModel in appmenu-gtk-module.

rilian added a comment.EditedFeb 14 2018, 3:29 PM

I cannot disable exporting an empty menubar, because exporting starts before realize (so, before menubar changes)

I do not know how to do it in GTK Wayland.

Can you explain me KDE Wayland Global Menu architecture?

broulik updated this revision to Diff 28255.Feb 28 2018, 10:51 AM
broulik edited the test plan for this revision. (Show Details)
  • Split icon mapping into dedicated namespace and extend it a lot
  • Monitor menus right away so we know if there's actually a menu (appmenu-gtk-module always claims to have a menu even if there is none)
  • Expand sections on the fly so ID mapping is correct and updating actions works ("Undo" action in LibreOffice updates fine now)
  • Fall back from menu bar to application menu on the fly (appmenu-gtk-module always announces a menu bar even if the app might only have an app menu)
  • Let "items to be added" also create new sections, fixes switching from LibreOffice Splash to Writer where the menu is replaced entirely
  • Fix updating visible/enabled property of actions at runtime
  • A couple of sanity checks and crash fixes
broulik updated this revision to Diff 28256.Feb 28 2018, 11:22 AM
  • Support radio buttons
  • Fix unchecking check boxes
broulik edited the summary of this revision. (Show Details)Feb 28 2018, 1:18 PM
broulik updated this revision to Diff 28320.Mar 1 2018, 12:49 PM
  • Clean up
  • Split Action and Menu handling in their dedicated classes
  • Ignore non-"Normal" windows, e.g. dialogs and toolboxes
mart accepted this revision as: mart.Mar 1 2018, 2:49 PM
mart added a subscriber: mart.

looks good to me, partial shipit, is so big that i think is better if another one reviews it too

gmenu-dbusmenu-proxy/window.cpp
289

always 2? maybe needs a more in depth comment?

This revision is now accepted and ready to land.Mar 1 2018, 2:49 PM
davidedmundson requested changes to this revision.Mar 6 2018, 12:06 PM
davidedmundson added a subscriber: davidedmundson.

Fundamental design seems fine.

It's a massive patchset, I'm not sure I've got my head round all of it yet, I might take a second round when these minor things are addressed.

gmenu-dbusmenu-proxy/actions.cpp
63

leak

120

leak

gmenu-dbusmenu-proxy/gmenudbusmenuproxy.desktop
8

Why 0?

gmenu-dbusmenu-proxy/icons.cpp
25

does this list come from anywhere?

gmenu-dbusmenu-proxy/menu.cpp
99

watcher leaked

128

and again...

gmenu-dbusmenu-proxy/menuproxy.cpp
196

are these always set before the window is created?

250

what's this about?

gmenu-dbusmenu-proxy/window.cpp
93

who owns this?

358

I think it's safe, you'd have definitely added it before we process someone introspecting the path.

But why write it one way round and add a question when we can just move it.

607

so do so?

This revision now requires changes to proceed.Mar 6 2018, 12:06 PM
broulik marked 5 inline comments as done.Mar 6 2018, 2:30 PM
broulik added inline comments.
gmenu-dbusmenu-proxy/icons.cpp
25

It was done trial and error by running couple of gtk apps (gedit, gimp, inkscape, shotwell) etc, the kde 4 appmenu also had something like this albeit less elaborate

gmenu-dbusmenu-proxy/menuproxy.cpp
196

I haven't seen it not working and wasn't really keen on playing with native event filters again as I did in appmenu applet

250

KDE stuff shows up in xprop as

_KDE_NET_WM_APPMENU_OBJECT_PATH(STRING) = "/MenuBar/1"

note the STRING whereas GTK shows up as

_GTK_APPLICATION_OBJECT_PATH(UTF8_STRING)

note the UTF8_STRING. I did not find the corresponding type enum anywhere and it also seems to be different for different machines

broulik updated this revision to Diff 28836.Mar 6 2018, 2:31 PM
  • Fix leaks
  • Cleanup
broulik updated this revision to Diff 28838.Mar 6 2018, 2:45 PM

Fix UTF8_STRING atom type check

davidedmundson accepted this revision.Mar 9 2018, 2:23 PM
This revision is now accepted and ready to land.Mar 9 2018, 2:23 PM
broulik closed this revision.Mar 9 2018, 2:57 PM
Pitel added a subscriber: Pitel.Mar 15 2018, 6:13 AM