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

[René J.V. Bertin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120355/#review67533
-----------------------------------------------------------


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;
    }
}```

- René J.V. Bertin


On Sept. 26, 2014, 7:28 p.m., 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, 7:28 p.m.)
> 
> 
> 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/1f15b712/attachment.htm>