helloSystem/Menu

Make it work with the default Qt platform theme (QGenericUnixTheme), too

Opened this issue · 7 comments

probonopd commented

https://blog.broulik.de/2016/10/global-menus-returning/

KDE introduces _KDE_NET_WM_APPMENU_OBJECT_PATH and _KDE_NET_WM_APPMENU_SERVICE_NAME with the result that with platform themes (including the default Qt one) which do not have those global menus don't work anymore.

Those of you familar with Wayland might notice that it uses global window IDs, which don’t exist in a Wayland world.

Is there a way to do without this? I would be fine with a solution that leaves Wayland broken, if we can get global menus to work with the default Qt Unix platform theme again.

davidedmundson commented

If someone is using QGenericUnixTheme on X11 it will call the legacy RegisterWindow(WId, ObjectPath) on the AppMenu host.

It's not as neat as tying it to the window, but it does have all the info you need, for you to maintain a cache.

  • 👍1
probonopd commented

Thanks for the hint davidedmundson. I take it that it is not implemented this way in KDE, which means that if I'd like to use the QGenericUnixTheme on X11 with KDE Plasma's global menu then global menus would not work?

davidedmundson commented

In KDE our daemon listens for that, then writes out the relevant properties onto the window. Not for QGenericUnixTheme but for GTK2 apps.

This way we keep our UIs just having one code path.

probonopd commented

We can tell Qt to keep using our platform plugin (helloSystem/QtPlugin) (which does the global menu stuff) but stil use another style like this:

# Use KDE default style
export QT_STYLE_OVERRIDE=Breeze

# Use Qt default style
export QT_STYLE_OVERRIDE=Fusion

If the objective is to just make it look like "normal", then this solves it.

However, one still needs our QtPlugin for the global menus to work since the normal Qt default one doesn't do it.

If one wants global menus to work with the default QGenericUnixTheme, then given @davidedmundson's explanations, I see two possible solutions:

  1. Hack on QGenericUnixTheme (or some other Qt component?) to make it support global menus by writing the relevant properties _KDE_NET_WM_APPMENU_OBJECT_PATH and _KDE_NET_WM_APPMENU_SERVICE_NAME (like we currently do in QtPlugin). The question is whether Qt would accept such a patch. This would satisfy the requirement of this ticket. (Downside: It wii only work for QGenericUnixTheme, not for all Qt themes.)
  2. Hack on KDE's daemon (probably this one?) to write the relevant properties onto the window not only for Gtk but also for Qt. This is even better because then it will work with any Qt theme, not just the default QGenericUnixTheme. The question is whether KDE would accept such a patch. (Downside: It will work only if KDE's daemon is running.)
  3. To not run into the mentioned downsides: Hack on something else in Qt? What would this "something" be? The xcb and Wayland platform integrations in Qt?

Also, somehow I'd see a much higher chance of this getting universally adopted if it didn't have a _KDE prefix. @davidedmundson would it be acceptable for KDE to also check for the presence of _NET_WM_APPMENU_OBJECT_PATH and _NET_WM_APPMENU_SERVICE_NAME and use them in case the ones prefixed with _KDE are not there`

probonopd commented

If someone is using QGenericUnixTheme on X11 it will call the legacy RegisterWindow(WId, ObjectPath) on the AppMenu host.

It's not as neat as tying it to the window, but it does have all the info you need, for you to maintain a cache.

@davidedmundson still struggling with this three years later. So you are saying one could observe D-Bus for RegisterWindow messages and somehow figure out the needed values for _KDE_NET_WM_APPMENU_OBJECT_PATH and _KDE_NET_WM_APPMENU_SERVICE_NAME from there? How exactly would that work?

% dbus-monitor 2>&1 | grep RegisterWindow

# Now launch an application

method call time=1694860093.982770 sender=:1.352 -> destination=com.canonical.AppMenu.Registrar serial=12 path=/com/canonical/AppMenu/Registrar; interface=com.canonical.AppMenu.Registrar; member=RegisterWindow
   string "No such method 'RegisterWindow' in interface 'com.canonical.AppMenu.Registrar' at object path '/com/canonical/AppMenu/Registrar' (signature 'us')"

% xprop
_KDE_NET_WM_APPMENU_OBJECT_PATH(STRING) = "/MenuBar/1"
_KDE_NET_WM_APPMENU_SERVICE_NAME(STRING) = ":1.352"

So, something (Menu?) would need to implement RegisterWindow and do something with it?

Actually, we do have a void MenuImporter::RegisterWindow(WId id, const QDBusObjectPath &path)

Menu/src/appmenu/menuimporter.cpp

Lines 65 to 105 in 6f4f432

void MenuImporter::RegisterWindow(WId id, const QDBusObjectPath &path)
{
qDebug() << path.path() << __LINE__ << message().service();
KWindowInfo info(id, NET::WMWindowType, NET::WM2WindowClass);
NET::WindowTypes mask = NET::AllTypesMask;
QString service = message().service();
if (!m_serviceWatcher->watchedServices().contains(service) || m_menuPaths[id]!=path) {
if(!m_serviceWatcher->watchedServices().contains(service)) {
m_serviceWatcher->addWatchedService(service);
}
} else {
emit WindowRegistered(id, service, m_menuPaths[id]);
return;
}
// Menu can try to register, right click in gimp for example
if (info.windowType(mask) & (NET::Menu | NET::DropdownMenu | NET::PopupMenu)) {
return;
}
if (path.path().isEmpty()) // prevent bad dbusmenu usage
return;
QString classClass = info.windowClassClass();
m_windowClasses.insert(id, classClass);
m_menuServices.insert(id, service);
m_menuPaths.insert(id, path);
// Fix for Chrome and Firefox, thanks Jesper Schmitz Mouridsen (jsmdk)
if (path.path().startsWith(QStringLiteral("/com/canonical/menu"))
&& KWindowSystem::isPlatformX11()) {
auto *c = QX11Info::connection();
xcb_change_property(c, XCB_PROP_MODE_REPLACE, id,
get_xcb_atom(_KDE_NET_WM_APPMENU_OBJECT_PATH, c), XCB_ATOM_STRING, 8,
path.path().size(), qPrintable(path.path()));
xcb_change_property(c, XCB_PROP_MODE_REPLACE, id,
get_xcb_atom(_KDE_NET_WM_APPMENU_SERVICE_PATH, c), XCB_ATOM_STRING, 8,
service.length(), qPrintable(service));
}
emit WindowRegistered(id, service, path);
}

but is it not doing the right thing when the _KDE atoms are not present?

It really doesn't help that this Global Menu stuff is lacking a coherent, comprehensive specification and documentation. Bits and pieces exist(ed) at Canonical (now defunct?) and KDE, and it is not always really clear what is and isn't in active use and supported by everyone (hello UBUNTU_MENUPROXY variable).

probonopd commented

https://blog.broulik.de/2016/10/global-menus-returning/ starts out so promising

Since Qt 5.7, thanks to Dmitry Shachnev, QGenericUnixTheme out of the box supports exporting a QMenuBar over DBus and registering it to the com.canonical.AppMenu.Registrar. When I updated to said version I was disappointed that it didn’t “just work”.

So it's really sad that seven years and a Qt major version later, it still doesn't "just work".

Or are we doing it wrong @kbroulik?

jsm222 commented

Or are we doing it wrong @kbroulik?

Yes qt6 works fine, it registers its objectPath in registerWindow. We do not need KDE properties on windows, we just have to keep track of the path in some other way. BTW I am about to refactor the searching of menuitems. So basically instead of (slowly) traversing the menu over dbus I get the whole layout in one call, i.e

    QDBusPendingCall call = m_interface->asyncCall("GetLayout",0, -1, QStringList());
  • 👍1