Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
Thomas Lübking
thomas.luebking at gmail.com
Sat Sep 27 20:00:28 BST 2014
> On Sept. 27, 2014, 6:13 nachm., René J.V. Bertin wrote:
> > How about this modification if I were to modify `KMenu::addTitle` to do something similar to what my patch to Konqueror does, on OS X? I modified the code to detect when an action is being added to a KMenu that has a KMainWindow with a KMenuBar among its associated widgets. If I understand correctly, this means that the menu *may* end up being displayed in the global menubar on OS X. It's not as refined as I would have liked, but there appears to be no reliable way to detect whether a menu belongs to a menubar that is the OS X global one.
> >
> > ```C++
> > QAction* KMenu::addTitle(const QIcon &icon, const QString &text, QAction* before)
> > {
> > bool notMacMenuBar = true;
> > #ifdef Q_OS_MAC
> > if (QAction *mAct = menuAction()) {
> > qDebug() << "## addTitle this=" << this << "mAct=" << mAct << mAct->text();
> > foreach (QWidget *w, mAct->associatedWidgets()) {
> > qDebug() << "### widget" << w << w->windowTitle() << "parent=" << w->parentWidget();
> > if (qobject_cast<KMenu*>(w) || qobject_cast<QMenu*>(w)) {
> > if (KMainWindow *obj = qobject_cast<KMainWindow*>(w->parentWidget())) {
> > if (obj->hasMenuBar()) {
> > // this is a KMainWindow with a menubar. On OS X, that could be the menubar, in which we
> > // have to create our title items differently.
> > notMacMenuBar = false;
> > qDebug() << "#### widget" << obj << obj->windowTitle() << "has menubar" << obj->menuBar() << "with parent" << obj->menuBar()->parentWidget();
> > break;
> > }
> > }
> > }
> > }
> > }
> > #endif // Q_OS_MAC
> > if (notMacMenuBar) {
> > QAction *buttonAction = new QAction(this);
> > QFont font = buttonAction->font();
> > font.setBold(true);
> > buttonAction->setFont(font);
> > buttonAction->setText(text);
> > buttonAction->setIcon(icon);
> >
> > QWidgetAction *action = new QWidgetAction(this);
> > action->setObjectName(KMENU_TITLE);
> > QToolButton *titleButton = new QToolButton(this);
> > titleButton->installEventFilter(d); // prevent clicks on the title of the menu
> > titleButton->setDefaultAction(buttonAction);
> > titleButton->setDown(true); // prevent hover style changes in some styles
> > titleButton->setToolButtonStyle(Qt::ToolButtonTextBesideIcon);
> > action->setDefaultWidget(titleButton);
> >
> > insertAction(before, action);
> > return action;
> > }
> > else{
> > QAction *action = new QAction(this);
> > action->setText(text);
> > action->setIcon(icon);
> > action->setEnabled(false);
> > if (before && actions().contains(before)) {
> > QAction *sepLow = new QAction(this);
> > sepLow->setSeparator(true);
> > insertAction(before, sepLow);
> > insertAction(sepLow, action);
> > if (!actions().startsWith(action)) {
> > QAction *sepHigh = new QAction(this);
> > sepHigh->setSeparator(true);
> > insertAction(action,sepHigh);
> > qDebug() << "#### inserted high separator before" << action << "before low separator before" << before;
> > }
> > else{
> > qDebug() << "#### inserted" << action << "before low separator before" << before;
> > }
> > }
> > else{
> > addAction(action);
> > addSeparator();
> > qDebug() << "#### appended low separator after" << action << "after existing" << actions().size()-2 << "items (before=" << before;
> > }
> > return action;
> > }
> > }```
While it's true that my sketch simplified in ignoring the possibility to have the title on a submenu to the global menubar, this patch is *very* specific in it's matching (though not necessarily correct)
It matches a popup menu which is submenu to a popup menu which is parented by a KMainWindow which has a menubar...
There're also some technical issues, but that's minor.
In any case a strongly suggest to open a review request for this patch to be annotated.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120355/#review67533
-----------------------------------------------------------
On Sept. 26, 2014, 5:28 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. 26, 2014, 5:28 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
>
>
> File Attachments
> ----------------
>
> patch for qwidget_mac.mm
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/b5c2dd92-33db-4225-9750-d10e13f0f835__prevent_addTitleRelated_crash.patch
> with the Qt patch
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/96f4fbfa-854e-4596-9f5f-d82f98a06955__Screen_shot_2014-09-26_at_19.16.20.png
> with the addTitle emulation patch
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/5ddf4a63-b3bb-415a-815a-c06eb7a5c7f2__Screen_shot_2014-09-26_at_19.19.40.png
>
>
> Thanks,
>
> René J.V. Bertin
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140927/1f000798/attachment.htm>
More information about the kde-core-devel
mailing list