Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
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 eresli...@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);
}
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/ --- (Updated Oct. 2, 2014, 7:10 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 (updated) 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 Blimey, instead of a crash I get almost the intended addTitle effect! And yes, the Quit action has been moved by Qt... https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/d9f45b6f-f4dd-4ba8-93f9-efa798695f3f__Screen_shot_2014-10-02_at_19.05.34.png Thanks, René J.V. Bertin
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
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.
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 eresli...@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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
---
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_castKMenu*(w) || qobject_castQMenu*(w)) {
if (KMainWindow *obj =
qobject_castKMainWindow*(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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
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_castKMenu*(w) || qobject_castQMenu*(w)) {
if (KMainWindow *obj =
qobject_castKMainWindow*(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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 24, 2014, 7:48 p.m., 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- 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. Changes --- Indeed, adding a simple check against q-parentWidget() in QWidgetPrivate::setGeometry_sys_helper helps prevent the application crash when the `KMenu::addTitle` is called. That's good and well, but the end result is that the added title doesn't appear at all. Compare the 1st screenshot with the 2nd (in which the addTitle emulation under review here is active). 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 (updated) 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- 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. - 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 26, 2014, 5:40 nachm., 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. 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 --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 26, 2014, 5:40 nachm., 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); } 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); } ``` - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
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); } ``` 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. - 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 26, 2014, 5:40 nachm., 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. 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. - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/ --- (Updated Sept. 25, 2014, 4 p.m.) Review request for KDE Base Apps, KDE Software on Mac OS X, kdelibs, and Qt KDE. Changes --- added the qt-kde group 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
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.
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)
- 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 24, 2014, 7:48 p.m., 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)
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.
- René J.V.
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120355/#review67379
---
On Sept. 25, 2014, 4 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. 25, 2014, 4 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 24, 2014, 7:48 p.m., 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
Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/ --- Review request for KDE Base Apps, KDE Software on Mac OS X and kdelibs. 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67379 --- 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) - Thomas Lübking On Sept. 24, 2014, 5:10 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. 24, 2014, 5:10 nachm.) Review request for KDE Base Apps, KDE Software on Mac OS X and kdelibs. 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 24, 2014, 7:48 p.m., 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) 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. ;) - René J.V. --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67379 --- On Sept. 24, 2014, 7:10 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. 24, 2014, 7:10 p.m.) Review request for KDE Base Apps, KDE Software on Mac OS X and kdelibs. 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
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. ;) 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) - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67379 --- On Sept. 24, 2014, 5:10 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. 24, 2014, 5:10 nachm.) Review request for KDE Base Apps, KDE Software on Mac OS X and kdelibs. 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
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
On Sept. 24, 2014, 7:48 p.m., 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) 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. - René J.V. --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67379 --- On Sept. 24, 2014, 7:10 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. 24, 2014, 7:10 p.m.) Review request for KDE Base Apps, KDE Software on Mac OS X and kdelibs. 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
