[KDE/Mac] Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
René J.V. Bertin
rjvbertin at gmail.com
Thu Oct 2 17:07:26 UTC 2014
> On Sept. 26, 2014, 7:40 p.m., René J.V. Bertin wrote:
> > changing this patch to prevent just the call to invalidateBuffer_resizeHelper (and setting isResize=false when that function cannot be called) shows something in place of the menu's title exactly once. All other times the menu is opened, empty space is shown.
>
> Thomas Lübking wrote:
> No idea why you want to manipulate the resize flag, try just:
>
> diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm
> index f58a755..d6a2741 100644
> --- a/src/gui/kernel/qwidget_mac.mm
> +++ b/src/gui/kernel/qwidget_mac.mm
> @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, int y, int w, int h, bool isM
>
> setWSGeometry(false, oldRect);
>
> - if (isResize && QApplicationPrivate::graphicsSystem())
> + if (isResize && QApplicationPrivate::graphicsSystem() && q->parentWidget())
> invalidateBuffer_resizeHelper(oldp, olds);
> }
>
> Thomas Lübking wrote:
> blast.
>
> ```diff
> diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm
> index f58a755..d6a2741 100644
> --- a/src/gui/kernel/qwidget_mac.mm
> +++ b/src/gui/kernel/qwidget_mac.mm
> @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, int y, int w, int h, bool isM
>
> setWSGeometry(false, oldRect);
>
> - if (isResize && QApplicationPrivate::graphicsSystem())
> + if (isResize && QApplicationPrivate::graphicsSystem() && q->parentWidget())
> invalidateBuffer_resizeHelper(oldp, olds);
> }
> ```
>
> René J.V. Bertin wrote:
> let's say that I unset isResize so that the rest of the function can finish in a slightly more appropriate fashion. I don't think it'd do to let it behave as if the resize helper did its job when that function hasn't been called.
>
> Thomas Lübking wrote:
> It did the resize in a reasonable fashion - at "worst" i't required to follow the function with isRealWindow, ie. have qt_mac_update_sizer() called, but the resize event very most likely needs to be sent.
>
> René J.V. Bertin wrote:
> Thomas, I thought I'd make a pure Qt example to add to a bug report on qt-projects.org, so I copied over `KMenu::addTitle` and the minimum required stuff from `KMenuPrivate` into a Qt example (systray):
>
> ```C++
> class KMenuPrivate
> : public QObject
> {
> public:
> KMenuPrivate (QMenu *_parent)
> {
> parent = _parent;
> }
>
> /**
> * @internal
> *
> * This event filter which is installed
> * on the title of the menu, which is a QToolButton. This will
> * prevent clicks (what would change down and focus properties on
> * the title) on the title of the menu.
> *
> * @author Rafael Fernández López <ereslibre at kde.org>
> */
> bool eventFilter(QObject *object, QEvent *event)
> {
> if (event->type() == QEvent::Paint ||
> event->type() == QEvent::KeyPress ||
> event->type() == QEvent::KeyRelease) {
> qDebug() << "Menu" << parent << parent->title() << "rejecting event" << event << "for" << object << object->objectName();
> return false;
> }
>
> event->accept();
> return true;
> }
>
> QMenu *parent;
> };
>
> QAction* qMenuAddTitle(QMenu *menu, const QIcon &icon, const QString &text, QAction* before=NULL)
> {
> QAction *buttonAction = new QAction(menu);
> QFont font = buttonAction->font();
> font.setBold(true);
> buttonAction->setFont(font);
> buttonAction->setText(text);
> buttonAction->setIcon(icon);
>
> QWidgetAction *action = new QWidgetAction(menu);
> action->setObjectName("KMENU_TITLE");
> QToolButton *titleButton = new QToolButton(menu);
> titleButton->installEventFilter(new KMenuPrivate(menu)); // 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);
>
> menu->insertAction(before, action);
> return action;
> }
>
> //...
>
> void Window::addActions( QMenu *menu )
> {
> qMenuAddTitle(menu, QIcon(), "Window actions");
> menu->addAction(minimizeAction);
> menu->addAction(maximizeAction);
> menu->addAction(restoreAction);
> // menu->addSeparator();
> qMenuAddTitle(menu, QIcon(), "Lethal action");
> menu->addAction(quitAction);
> }
>
> void Window::createTrayIcon()
> {
> trayIconMenu = new QMenu(this);
> addActions(trayIconMenu);
>
> trayIcon = new QSystemTrayIcon(this);
> trayIcon->setContextMenu(trayIconMenu);
>
> if( (menuBar = new QMenuBar(NULL)) && (standardMenu = menuBar->addMenu(tr("a Menu"))) ){
> addActions(standardMenu);
> }
> }
> ```
>
> I then reinstalled an unpatched Qt4. And guess what ... no crash. I presume you have no idea what KDE might do differently that KDE apps crash when invoking `insertAction` in `addTitle`?!
>
> Thomas Lübking wrote:
> Random guess: pre ./. post eventloop generation?
> Try to make addActions() a slot and bind it to the QMenu aboutToShow() signal. (You'll need "QMenu *menu = qobject_cast<QMenu*>(sender());" since ::addActions() must not have a parameter for signature match)
Like so?
```C++
void Window::addActions()
{
QMenu *menu = qobject_cast<QMenu*>(sender());
menu->clear();
qMenuAddTitle(menu, QIcon(), "Window actions");
menu->addAction(minimizeAction);
menu->addAction(maximizeAction);
menu->addAction(restoreAction);
// menu->addSeparator();
qMenuAddTitle(menu, QIcon(), "Lethal action");
menu->addAction(quitAction);
}
void Window::createTrayIcon()
{
trayIconMenu = new QMenu(this);
connect( trayIconMenu, SIGNAL(aboutToShow()), this, SLOT(addActions()) );
trayIcon = new QSystemTrayIcon(this);
trayIcon->setContextMenu(trayIconMenu);
if( (menuBar = new QMenuBar(NULL)) && (standardMenu = menuBar->addMenu(tr("a Menu"))) ){
connect( standardMenu, SIGNAL(aboutToShow()), this, SLOT(addActions()) );
}
}
```
Still no crash, but now the menu actually shows the QToolButton that's being used (with the wrong font and not nicely aligned, but it'd be a start if this is what I were after!)
- René J.V.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120355/#review67495
-----------------------------------------------------------
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-mac/attachments/20141002/b52c49a5/attachment-0001.html>
More information about the kde-mac
mailing list