(And to reply to the ineviatible Kai question, no this doesn't fix all the update problems - but it does put us one step closer)

Hi David,

You have an interesting approach here!
I've been struggling with something similar recently as well and also - initially - had my own lookup table for int -> QAction. It worked, but the added bookkeeping seemed silly so i searched for a more "elegant" solution.
I ended up doing the bookkeeping in QAction itself.
This is O(n) complexity, not O(1), but it saves having to maintain your own bookkeeping for actions in a menu. That can't ever be so dreadfully big to slow you down so i think you're safe with this approach.

What you would need to do for this:

1. Get rid of the bookkeeping, you don't need them.

typedef QMap<int, QAction* > ActionForId;
ActionForId m_actionForId;

2. The remove action part becomes something like this:

(note: why do you mix foreach and Q_FOREACH? pick one or the other)

foreach (QAction *action, menu->actions()) {
    int id = action->property(DBUSMENU_PROPERTY_ID).toInt();
    Q_FOREACH(const DBusMenuLayoutItem &dbusMenuItem, rootItem.children) {
        if (dbusMenuItem.id == id) {
            action->deleteLater();
            break;
        }
    }
}

3. The add action becomes something like:

Q_FOREACH(const DBusMenuLayoutItem &dbusMenuItem, rootItem.children) {

    auto it = std::find_if(d->m_actionForId.cbegin(), d->m_actionForId.cend(), [&dbusMenuItem](QAction *action)
    {
       return action->property(DBUSMENU_PROPERTY_ID).toInt() == dbusMenuItem.id;
    });

    QAction *action = nullptr;
    if (it == d->m_actionForId.end()) {
        int id = dbusMenuItem.id;
        action = d->createAction(id, dbusMenuItem.properties, menu);
        d->m_actionForId.insert(id, action);

        connect(action, &QAction::triggered, this, [action, id, this]() {
            sendClickedEvent(id);
        });

        menu->addAction(action);
    } else {
        action = *it;
        d->updateAction(*it, dbusMenuItem.properties, dbusMenuItem.properties.keys());
    }
}

Not tested and guessed some code.. But you get the point i think.
Also, i seem to be missing where you set DBUSMENU_PROPERTY_ID on an action (or it's done in d->createAction?).

That's more elegant, right?

Good luck :)

Also, i seem to be missing where you set DBUSMENU_PROPERTY_ID on an action (or it's done in d->createAction?).

Yes, it's an existing thing set there.

The map is also existing, and it's used for a few other entry points, especially DBusMenuImporter::getMenuById()

I would do your thing in (3), but given this map is already needed, I may as well use it.

With this updated patch I no longer get submenus in window decoration, with the previous revision it worked.