> 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.
> 
> Thomas Lübking wrote:
>     > 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.
> 
> René J.V. Bertin wrote:
>     It's late here too - you think there's another workaround?

I'd skip efforts to work-a-round and fix qwidget_mac.mm instead.
This bug threatens beyond adding titles to menus and even widget actions.

*Everytime* a parentless, but "embedded" (didn't look into the mac specific 
code) widget is resized, you'll get a crash.
The buffer invalidation does only make sense for parented widgets and will 
inevitably crash (there's even a Q_ASSERT for it) without.


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

Reply via email to