Bug 369276

Summary:	KActionCollection, menu/action reuse and the native Mac menubar
Product:	[Frameworks and Libraries] frameworks-kxmlgui	Reporter:	RJVB <rjvbertin>
Component:	general	Assignee:	kdelibs bugs <kdelibs-bugs>
Status:	CLOSED UPSTREAM
Severity:	major	CC:	christoph, faure, mk-lists
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Other
OS:	macOS
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description RJVB 2016-09-24 08:45:01 UTC

I'm making another attempt at raising awareness about a Qt platform limitation on Mac that has potentially severe implications for reuse of QMenus, QActions and QActionWidgets in multiple menus if one of those is attached to a native QMenuBar.

In short, this is not possible (because items attached to the native menubar are reparented away from their original parent), and according to the documentation it can lead to the menu items (be they actions or submenus) not appearing or remaining disabled. The documentation isn't explicit which instances might not appear or remain disabled; practical experience suggests this only occurs in menus under the toplevel menubar.
Qt versions before 5.3 were not affected to the best of my knowledge; 5.4 and 5.5 would print error messages each time the software added an item to or removed it from an additional menu.

In other words, the KActionCollection class is compromised on OS X, and that's not exactly a class that sees only limited use.

The big problem here is that it is near impossible to predict which instances will be affected. With earlier KTextEditor and Qt versions, for instance, I had to disable the paste history and bookmarks menus to stop the error messages. With Qt 5.6.1 and KTextEditor 5.24.0 it is the Mode and Highlighting menus that are populated almost exclusively with disabled submenus.

The fact that items are reparented suggests that reusing them may also lead to memory leak, accessing stale objects or double freeing (and indeed I have seen some crashes that appeared related to menu item reuse).

The most effective solution I've found until now is to disable the native menubar. Evidently this isn't ideal if the goal is to make KDE applications look and behave as natively as possible. Also, just setting the Qt::AA_DontUseNativeMenuBar attribute before any menus are created isn't enough for all applications. Some may (like KHelpCenter) never show the menubar, some (those that do not offer a hide/show menu feature like KDevelop) may start unpredictably with the menubar in hidden state, or hide it at runtime in reaction to some event.

The alternative solution is to avoid reuse on OS X, either in the source code or in the ui.rc definition files but it is my experience that it isn't always trivial to figure out what to disable where.

I've tried to get some answers about the underlying limitation from the Qt team on one of their mailing lists, but it is probably worthwhile to start thinking of workarounds independently. After all the official Qt stance may be "software just shouldn't do this", like with QMenu::addSection() (which doesn't add a named section on all platforms).

Reproducible: Sometimes

Steps to Reproduce:
1. Create QMenu, QAction and/or QActionWidget instances
2. Add them to a QMenuBar that corresponds to the Mac native, toplevel menubar in addition to one or more other menu(bar)s

Actual Results:
the instances (or items therein) may not appear or remain disabled, a priori only in the toplevel menubar (but that may depend on the order of attribution).

Expected Results:
All instances in all menu(bar)s appear and are enabled as expected

I'm hesitating between calling this a wishlist ticket and something stronger. KActionCollection sees considerably widespread use, so I'm going to settle with "Major".

Comment 1 David Faure 2016-11-06 09:35:08 UTC

The basic QAction concept is that you can add the same action to multiple widgets.
If this doesn't work in some cases, it's a Qt bug, not a problem in KActionCollection (which is really just a container of qactions). 

=> report this on the Qt bug tracker. Preferrably with an actual testcase.

Comment 2 RJVB 2016-11-06 12:46:00 UTC

The thing is that their documentation makes it clear that you cannot add a QAction to multiple menus if one of them corresponds to a menu item in a menu that's part of the native menubar.
I've already tried to discuss this on the Qt MLs (development, I think), but that didn't lead anywhere. Which makes me think that they don't consider this a bug, simply a limitation that we have to live with. That's also how they treated the bug report about "texted" menu sections which don't work on OS X that way: just don't use them.
The only difference here is that in this case one can make a point that the limitation didn't exist in Qt4, and thus is a feature regression.

So no, this issue is not resolved (I'll leave it closed though).

Comment 3 David Faure 2016-11-06 19:40:09 UTC

Amazing.

So a QAction in both a toolbar and a menu in the native menubar is OK?
Or as soon as a QAction is in a native menu (short for "menu in the native menubar") it can't be used anywhere else? This totally defeats the QAction concept, asking apps to create multiple QActions and keep them in sync. We're back to Qt 2 then...

If you open a Qt bug report I can weigh in about how unacceptable this is ;)

Comment 4 RJVB 2016-11-06 20:48:00 UTC

I guess this depends on exactly what you use it for in the toolbar, but yes, it's possible.

The problem is a bit that I'm not perfectly sure exactly which classes are concerned. The last time I've seen a real problem arise was with Kate's (and thus KDevelop's) Mode and Highlighting menus. IIRC those are populated with QAction instances; the symptom was that most items were disabled and some never appeared. That corresponds to what is written in the docs for *QWidgetAction* :

OS X: If you add a widget to a menu in the application's menu bar on OS X, the widget will be added and it will function but with some limitations:
1. The widget is reparented away from the QMenu to the native menu view. If you show the menu in some other place (e.g. as a popup menu), the widget will not be there.

What makes this extra annoying is that it is hard to predict exactly what happens. The claim above ("will not be there") is not necessarily true for instance, at least not always. Usually I've been able to see the items in both the native menu and a context menu, for instance. And the Mode/Highlighting issue in Kate weren't affected from my first attempts with KF5 (Kate was the 1st app I built). They worked, but the Bookmark menu didn't; the Mode and Highlighting menus were affected only recently but the code didn't or hardly change. IOW, the actual effects of the reparenting *may* depend on the order in which things happen, which could depend on changes in related code (kxmlgui..) or even compiler options (I think).

I'll try to find some time tomorrow to see exactly what was being added to those Kate menus that remained disabled (but I take it that the limitation would be almost just as unacceptable for QWidgetActions?).

Comment 5 RJVB 2016-11-07 09:18:59 UTC

So, I should have reread my original report...

The Mode and Highlighting menus are provided by the KTextEditor framework. They're both KActionMenu instances, so QWidgetActions. However, they are populated with QAction* instances, and it's those instances that are problematic. The Mode and Highlighting menus themselves appear and appear to work as they should in themselves.

Distilling this into a simple test case seems a rather daunting task ...