Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu

[Thomas Lübking]

> 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.

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)


- 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/e2cec312/attachment.htm>