Singleton of Actions do not work with multiple windows of same application

[Czaki]

In napari we got from users information that menus disappear if user close latest created window.

napari/napari#7588
napari/napari#8024

My investigation shows that it looks to be caused to a singleton nature of Actions in app-model.

app-model/src/app_model/backends/qt/_qaction.py

Lines 178 to 182 in 06c1bc4

	cache_key = QMenuItemAction._cache_key(app, menu_item)
	if cache_key in cls._cache:
	res = cls._cache[cache_key]
	res.setParent(parent)
	return res

As the latest created window is set to be a parent of actions, then, on close of this window, Qt destroys of actions, so the actions effectively disappear from all windows.

What is the motivation to use singleton of QMenuItemAction, when QModelSubmenu are not singletons?

If it is for update, shouldn't we store them per QMainWindow instance?

[tlambert03]

thanks for opening, let's figure out a fix...

As the latest created window is set to be a parent of actions, then, on close of this window, Qt destroys of actions, so the actions effectively disappear from all windows.

my first question was indeed going to be about cleanup. if there's no way to make these QActions "application wide" (rather than window specific) then yes indeed, we'll need to fix that parenting.

The motivation for making actions singletons is that it makes it much easier for the same action to be represented in, say, a toolbutton, as well as in multiple different menu locations, while having its enablement and toggle state remain in sync. Naturally, we could achieve that some other way, but whatever alternative solution we come to, we should make sure that we don't create multiple QActions representing the same app-model action that can potentially go out of sync.

If it is for update, shouldn't we store them per QMainWindow instance?

That would certainly be what napari needs (and I'm definitely open to doing that if it fixes your proximal need) ... however, I do think that napari has a bit of a funny "application" model. While you see each of your windows as an entire application unto itself, I think many other QApplication-based projects would have a bit more of a global app concept, where certain state is more "session-related" rather than "window-related". That said, in most of those cases, there would only be one QMainWindow instance anyway.

If you'd like to propose a way to build QActions for each QMainWindow rather than for the app on the whole, that would be fine with me!

[Czaki]

my first question was indeed going to be about cleanup. if there's no way to make these QActions "application wide" (rather than window specific) then yes indeed, we'll need to fix that parenting.

In my opinion, these actions are application wide, but QMenuModel:

app-model/src/app_model/backends/qt/_qmenu.py

Line 54 in 06c1bc4

self.rebuild()

app-model/src/app_model/backends/qt/_qmenu.py

Lines 309 to 311 in 06c1bc4

	elif item.command.id not in _exclude:
	action = QMenuItemAction.create(item, app=app, parent=menu)
	menu.addAction(action)

Is adding/updating the parent to these actions. But When I remove parent=menu argument no action is present (I expect that there is no reference to keep them alive).

But when I set QApplication.instance() as parent it looks working. What did you think?

[Czaki]

The other option is to store hard references in QMenuModel instance, so once all windows are closed, the actions will be destroyed.

[tlambert03]

QMenuModel Is adding/updating the parent to these actions

yeah, I don't recall why i would have done it that way... perhaps because of what you observed...
however, I'm plenty happy with the QApplication.instance() as the parent if you are?