Fall back from menu bar to application menu on the fly

appmenu-gtk-module always announces a menu bar even if there is none, so we always
request a menu bar and when it turns out to be empty and have actually have an app menu
to fall back to we'll use that instead.

gmenu-dbusmenu-proxy/menu.cpp

@@ -56,15 +56,24 @@ Menu::~Menu() = default;
 
 void Menu::init()
 {
-     qCDebug(DBUSMENUPROXY) << "Inited menu for" << m_winId << "on" << m_serviceName << "at app" << m_applicationObjectPath << "win" << m_windowObjectPath << "unity" << m_unityObjectPath << "menu" << m_menuObjectPath;
+     qCDebug(DBUSMENUPROXY) << "Inited menu for" << m_winId << "on" << m_serviceName << "at app" << m_applicationObjectPath << "win" << m_windowObjectPath << "unity" << m_unityObjectPath;
+
+     if (!QDBusConnection::sessionBus().connect(m_serviceName,
+                                                m_applicationMenuObjectPath,
+                                                s_orgGtkMenus,
+                                                QStringLiteral("Changed"),
+                                                this,
+                                                SLOT(onApplicationMenuChanged(GMenuChangeList)))) {
+         qCWarning(DBUSMENUPROXY) << "Failed to subscribe to application menu changes on" << m_serviceName << "at" << m_applicationMenuObjectPath;
+     }
 
     if (!QDBusConnection::sessionBus().connect(m_serviceName,
-                                               m_menuObjectPath,
+                                               m_menuBarObjectPath,
                                                s_orgGtkMenus,
                                                QStringLiteral("Changed"),
                                                this,
-                                               SLOT(onMenuChanged(GMenuChangeList)))) {
+                                               SLOT(onMenuBarChanged(GMenuChangeList)))) {
-        qCWarning(DBUSMENUPROXY) << "Failed to subscribe to menu changes on" << m_serviceName << "at" << m_menuObjectPath;
+        qCWarning(DBUSMENUPROXY) << "Failed to subscribe to menu bar changes on" << m_serviceName << "at" << m_menuBarObjectPath;
     }
 
     if (!m_applicationObjectPath.isEmpty() && !QDBusConnection::sessionBus().connect(m_serviceName,
@@ -178,6 +187,26 @@ void Menu::setUnityObjectPath(const QString &unityObjectPath)
     m_unityObjectPath = unityObjectPath;
 }
 
+QString Menu::applicationMenuObjectPath() const
+{
+    return m_applicationMenuObjectPath;
+}
+
+void Menu::setApplicationMenuObjectPath(const QString &applicationMenuObjectPath)
+{
+    m_applicationMenuObjectPath = applicationMenuObjectPath;
+}
+
+QString Menu::menuBarObjectPath() const
+{
+    return m_menuBarObjectPath;
+}
+
+void Menu::setMenuBarObjectPath(const QString &menuBarObjectPath)
+{
+    m_menuBarObjectPath = menuBarObjectPath;
+}
+
 QString Menu::windowObjectPath() const
 {
     return m_windowObjectPath;
@@ -188,14 +217,9 @@ void Menu::setWindowObjectPath(const QString &windowObjectPath)
     m_windowObjectPath = windowObjectPath;
 }
 
-QString Menu::menuObjectPath() const
+QString Menu::currentMenuObjectPath() const
 {
-    return m_menuObjectPath;
+    return m_currentMenuObjectPath;
-}
-
-void Menu::setMenuObjectPath(const QString &menuObjectPath)
-{
-    m_menuObjectPath = menuObjectPath;
 }
 
 QString Menu::proxyObjectPath() const
@@ -226,8 +250,18 @@ void Menu::start(uint id)
 
     // dbus-send --print-reply --session --dest=:1.103 /org/libreoffice/window/104857641/menus/menubar org.gtk.Menus.Start array:uint32:0
 
+    if (m_currentMenuObjectPath.isEmpty()) {
+        m_currentMenuObjectPath = m_menuBarObjectPath;
+    }
+
+    if (m_currentMenuObjectPath.isEmpty()) {
+        m_currentMenuObjectPath = m_applicationMenuObjectPath;
+    }
+
+    Q_ASSERT(!m_currentMenuObjectPath.isEmpty()); // we shouldn't have been created without one
+
     QDBusMessage msg = QDBusMessage::createMethodCall(m_serviceName,
-                                                      m_menuObjectPath,
+                                                      m_currentMenuObjectPath,
                                                       s_orgGtkMenus,
                                                       QStringLiteral("Start"));
     msg.setArguments({
@@ -239,7 +273,7 @@ void Menu::start(uint id)
     connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, id](QDBusPendingCallWatcher *watcher) {
         QDBusPendingReply<GMenuItemList> reply = *watcher;
         if (reply.isError()) {
-            qCWarning(DBUSMENUPROXY) << "Failed to start subscription to" << id << "on" << m_serviceName << "at" << m_menuObjectPath << reply.error();
+            qCWarning(DBUSMENUPROXY) << "Failed to start subscription to" << id << "on" << m_serviceName << "at" << m_currentMenuObjectPath << reply.error();
         } else {
             const auto menus = reply.value();
             for (auto menu : menus) {
@@ -248,7 +282,17 @@ void Menu::start(uint id)
 
             // LibreOffice on startup fails to give us some menus right away, we'll also subscribe in onMenuChanged() if neccessary
             if (menus.isEmpty()) {
-                qCWarning(DBUSMENUPROXY) << "Got an empty menu for" << id << "on" << m_serviceName << "at" << m_menuObjectPath;
+                qCWarning(DBUSMENUPROXY) << "Got an empty menu for" << id << "on" << m_serviceName << "at" << m_currentMenuObjectPath;
+
+                // appmenu-gtk-module always claims to have a menu bar even if it is empty
+                // so when we root menu is requested but it is empty AND we have an app menu (because otherwise LibreOffice breaks)
+                // then we will switch to using app menu instead
+                if (id == 0 && m_currentMenuObjectPath == m_menuBarObjectPath && !m_applicationMenuObjectPath.isEmpty()) {
+                    qCDebug(DBUSMENUPROXY) << "Using application menu instead";
+                    m_currentMenuObjectPath = m_applicationMenuObjectPath;
+                    start(id);
+                }
+
                 return;
             }
 
@@ -283,8 +327,11 @@ void Menu::start(uint id)
 
 void Menu::stop(const QList<uint> &ids)
 {
+    if (m_currentMenuObjectPath.isEmpty()) {
+        qCWarning(DBUSMENUPROXY) << "Cannot stop subscriptions for" << ids << "without menu object path";
+    }
     QDBusMessage msg = QDBusMessage::createMethodCall(m_serviceName,
-                                                      m_menuObjectPath,
+                                                      m_currentMenuObjectPath,
                                                       s_orgGtkMenus,
                                                       QStringLiteral("End"));
     msg.setArguments({
@@ -296,7 +343,7 @@ void Menu::stop(const QList<uint> &ids)
     connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, ids](QDBusPendingCallWatcher *watcher) {
         QDBusPendingReply<void> reply = *watcher;
         if (reply.isError()) {
-            qCWarning(DBUSMENUPROXY) << "Failed to stop subscription to" << ids << "on" << m_serviceName << "at" << m_menuObjectPath << reply.error();
+            qCWarning(DBUSMENUPROXY) << "Failed to stop subscription to" << ids << "on" << m_serviceName << "at" << m_currentMenuObjectPath << reply.error();
         } else {
             // remove all subscriptions that we unsubscribed from
             // TODO is there a nicer algorithm for that?
@@ -307,7 +354,25 @@ void Menu::stop(const QList<uint> &ids)
     });
 }
 
-void Menu::onMenuChanged(const GMenuChangeList &changes)
+void Menu::onApplicationMenuChanged(const GMenuChangeList &changes)
+{
+    if (m_currentMenuObjectPath != m_applicationMenuObjectPath) {
+        qCInfo(DBUSMENUPROXY) << "Application menu changed for" << m_serviceName << "on" << m_applicationMenuObjectPath << "although we're actually using" << m_currentMenuObjectPath;
+        return;
+    }
+    menuChanged(changes);
+}
+
+void Menu::onMenuBarChanged(const GMenuChangeList &changes)
+{
+    if (m_currentMenuObjectPath != m_menuBarObjectPath) {
+        qCInfo(DBUSMENUPROXY) << "Menu bar changed for" << m_serviceName << "on" << m_menuBarObjectPath << "although we're actually using" << m_currentMenuObjectPath;
+        return;
+    }
+    menuChanged(changes);
+}
+
+void Menu::menuChanged(const GMenuChangeList &changes)
 {
     QSet<uint> dirtyMenus;
     DBusMenuItemList dirtyItems;

gmenu-dbusmenu-proxy/menu.h

@@ -61,8 +61,13 @@ public:
     QString windowObjectPath() const;
     void setWindowObjectPath(const QString &windowObjectPath);
 
-    QString menuObjectPath() const;
+    QString applicationMenuObjectPath() const;
-    void setMenuObjectPath(const QString &menuObjectPath);
+    void setApplicationMenuObjectPath(const QString &applicationMenuObjectPath);
+
+    QString menuBarObjectPath() const;
+    void setMenuBarObjectPath(const QString &menuBarObjectPath);
+
+    QString currentMenuObjectPath() const;
 
     QString proxyObjectPath() const;
 
@@ -87,7 +92,8 @@ signals:
     void LayoutUpdated(uint revision, int parent);
 
 private slots:
-    void onMenuChanged(const GMenuChangeList &changes);
+    void onApplicationMenuChanged(const GMenuChangeList &changes);
+    void onMenuBarChanged(const GMenuChangeList &changes);
     void onApplicationActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added);
     void onUnityActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added);
     void onWindowActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added);
@@ -103,6 +109,7 @@ private:
     bool getAction(const QString &name, GMenuAction &action) const;
     void triggerAction(const QString &name, uint timestamp = 0);
 
+    void menuChanged(const GMenuChangeList &changes);
     void actionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added,
                         GMenuActionMap &actions, const QString &prefix);
 
@@ -121,7 +128,10 @@ private:
     QString m_applicationObjectPath;
     QString m_unityObjectPath;
     QString m_windowObjectPath;
-    QString m_menuObjectPath;
+    QString m_applicationMenuObjectPath;
+    QString m_menuBarObjectPath;
+
+    QString m_currentMenuObjectPath;
 
     QString m_proxyObjectPath; // our object path on this proxy app
 

gmenu-dbusmenu-proxy/menuproxy.cpp

@@ -167,21 +167,19 @@ void MenuProxy::onWindowAdded(WId id)
     }
 
     const QString serviceName = QString::fromUtf8(getWindowPropertyString(id, s_gtkUniqueBusName));
-    const QString applicationObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkApplicationObjectPath));
-    const QString unityObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_unityObjectPath));
-    const QString windowObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkWindowObjectPath));
 
     if (serviceName.isEmpty()) {
         return;
     }
 
-    QString menuObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkMenuBarObjectPath));
+    const QString applicationObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkApplicationObjectPath));
-    if (menuObjectPath.isEmpty()) {
+    const QString unityObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_unityObjectPath));
-        // try generic app menu
+    const QString windowObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkWindowObjectPath));
-        menuObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkAppMenuObjectPath));
+
-    }
+    const QString applicationMenuObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkAppMenuObjectPath));
+    const QString menuBarObjectPath = QString::fromUtf8(getWindowPropertyString(id, s_gtkMenuBarObjectPath));
 
-    if (menuObjectPath.isEmpty()) {
+    if (applicationMenuObjectPath.isEmpty() && menuBarObjectPath.isEmpty()) {
         return;
     }
 
@@ -190,7 +188,8 @@ void MenuProxy::onWindowAdded(WId id)
     menu->setApplicationObjectPath(applicationObjectPath);
     menu->setUnityObjectPath(unityObjectPath);
     menu->setWindowObjectPath(windowObjectPath);
-    menu->setMenuObjectPath(menuObjectPath);
+    menu->setApplicationMenuObjectPath(applicationMenuObjectPath);
+    menu->setMenuBarObjectPath(menuBarObjectPath);
     m_menus.insert(id, menu);
 
     connect(menu, &Menu::requestWriteWindowProperties, this, [this, menu] {