Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
Thomas Lübking
thomas.luebking-Re5JQEeQqe8AvxtiuMwx3w at public.gmane.org
Thu Sep 25 23:18:54 BST 2014
> On Sept. 24, 2014, 5:48 nachm., Thomas Lübking wrote:
> > I assume you'd be better off altering KMenu::addTitle() - or even patch Qt (QMenu on mach cannot deal w/ widget actions, at least if used on the global menubar)
>
> René J.V. Bertin wrote:
> I agree totally, but for that
>
> - I'd have to understand exactly what the addTitle does that makes Qt/Mac crash
> - Ideally I'd also know how to determine if the menu is in the global menubar or e.g. in a popup menu, where addTitle works perfectly fine. I think we'd want to preserve that because popup menus follow the selected style and not necessarily the OS X style.
>
> There's also the point that the addTitle (and addSection, IIRC) in Qt5 don't crash. They have other issues (IIRC you get just a separator, not the title text) but until now I've preferred to handle these crashes on a case-by-case basis.
>
> I admit, this RR was also made a bit with the idea of getting a discussion going about this issue. ;)
>
> Thomas Lübking wrote:
> Since KMenu is deprecated and the ::addTitle() implementation doesn't differ in KF5, either the applications have simply been ported away from KMenu or QWidgetAction was fixed in Qt5.
>
> To know why exactly this crashes for you, i'd need to see a backtrace (paste.kde.org) - Qt4 claimed QWidgetAction support on OSX' global menu - with some caveats.
> If QMenu::menuAction() is in the action list of the global menu - unfortunately, this menubar is parentless :-(
> Also there's no guarantee that this assignment won't change at some point in the future to any direction.
>
> > IIRC you get just a separator, not the title text
>
> What basically means that the QWidget(Action) reparenting doesn't work at all in Qt5 anymore (at best the linked out widget is just hidden)
>
>
> Disclaimer: I'm a bit biased here ;-)
> Imo using a QWidgetAction as title was a wrong design itfp - I proposed a Qt4 patch to use a leading and entitled separator instead, but it was rejected because not all styles did/do support texted separators. No idea whether that patch was revived for Qt5, never tested. (And, tbh, I don't know whether the native styles, ie. Win and Mac, support texted separators)
>
> René J.V. Bertin wrote:
> backtrace: http://paste.kde.org/pvnu8pgui
>
> If I recall correctly, Qt5.3's QMenu::addTitle and QMenu::addSection indeed call for what I think you mean with texted separators. And OS X will only render the separator for those. OS X 10.6 in any case, but I don't see why that would have changed in later versions.
>
> Thomas Lübking wrote:
> Thanks.
> QMenu::addTitle() does not exist in 5.3 and ::setTitle() refers to the menubar item text.
> ::addSection() might work (if the building loop was reversed, making a separator as first element possible ;-)
>
> On the crash:
> It occurs because QWidgetPrivate::setGeometry_sys_helper() in qwidget_mac.mm is not aware that the widget it operates on is a toplevel widget (and has no parent)
> This seems to be the "QMacNativeWidget(0);" "container" created in qmenu_mac.mm, QMenuPrivate::QMacMenuPrivate::addAction()
>
> Why it doesn't figure so, I don't know, but assume that in
>
> ```cpp
> bool QWidgetPrivate::isRealWindow() const
> {
> return q_func()->isWindow() && !topData()->embedded;
> }
> ```
>
> "topData()->embedded" will be true (so the return be false)
>
> René J.V. Bertin wrote:
> Hmm, it's been a while that I looked at that - when making kmail "not crash" because of the same reason on OS X. I never submitted a patch for that here because I noticed that kdepim git/master used new QMenu functions. I ported over QMenu::addSection to KMenu, and that's where I saw that a "texted separator" remains just a separator on OS X.
>
> Are you sure a menubar becomes the global menubar only when it doesn't have a parent? I seem to recall that the situation is a little bit more complex than that.
> Are you sure a menubar becomes the global menubar only when it doesn't have a parent? I seem to recall that the situation is a little bit more complex than that.
I can only quote the Qt docs on this:
> If you want all windows in a Mac application to share one menu bar, you must create a menu bar that does not have a parent. Create a parent-less menu bar this way:
> QMenuBar *menuBar = new QMenuBar(0);
> Note: Do not call QMainWindow::menuBar() to create the shared menu bar, because that menu bar will have the QMainWindow as its parent. That menu bar would only be displayed for the parent QMainWindow.
This has however nothing to do with the crash - it's the "QMacNativeWidget" which has no parent but is treated by ::setGeometry_sys_helper() as if it had.
The call to ::invalidateBuffer_resizeHelper() must only happen "if (q->parentWidget())" resp. "if (!q->isWindow())". Whether it's embedded somewhere doesn't matter.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120355/#review67379
-----------------------------------------------------------
On Sept. 25, 2014, 2 nachm., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120355/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2014, 2 nachm.)
>
>
> Review request for KDE Base Apps, KDE Software on Mac OS X, kdelibs, and Qt KDE.
>
>
> Repository: kde-baseapps
>
>
> Description
> -------
>
> Mac OS X cannot handle the formatting used for title menu items when it applies to items in the toplevel menu bar. An application calling KMenu::addTitle on such a menu item will crash immediately, somewhere deep in Qt.
>
> This patch works around that crash by emulating the addTitle effect.
>
> Curiously, the addTitle call that causes the crash when clicking on the Help menu concerns a submenu of an item of the Tools menu...
>
>
> Diffs
> -----
>
> konq-plugins/uachanger/uachangerplugin.cpp 5e2d094
>
> Diff: https://git.reviewboard.kde.org/r/120355/diff/
>
>
> Testing
> -------
>
> OS X 10.6.8 with kdelibs 4.14.1
>
>
> Thanks,
>
> René J.V. Bertin
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140925/a5a0fd03/attachment.htm>
More information about the kde-core-devel
mailing list