[KDE/Mac] 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 19:00:28 UTC 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-mac/attachments/20140927/1f000798/attachment-0001.html>


More information about the kde-mac mailing list