From a78d66743171557d79b16c08be775e3ac15bb4ef Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Thu, 5 Dec 2019 09:33:50 +0100 Subject: Fix positioning of dynamically populated QToolButton::MenuButtonPopup's in screen corners The existing code positioning the menu in QToolButtonPrivate::popupTimerDone() had a clause checking whether any receivers were connnected to QMenu::aboutToShow() causing the sizeHint to be -1,-1 in that case (apparently trying to accommodate menus populated in slots). In that case, the checking for screen borders would not work, causing the similar checks in QMenu::popup() to trigger, potentially positioning the menu over the mouse. To solve this dilemma, add a parameter taking a std::function calulating the position of the menu from the sizeHint to QMenuPrivate::exec()/popup() and invoke that in QMenuPrivate::popup() after emitting QMenu::aboutToShow() when the sizeHint is known. Fixes: QTBUG-78966 Change-Id: I180bd2dc7eadcaca6cadca13745ed4a2dd89e412 Reviewed-by: Volker Hilsheimer --- src/widgets/widgets/qmenu.cpp | 10 ++++-- src/widgets/widgets/qmenu_p.h | 8 +++-- src/widgets/widgets/qtoolbutton.cpp | 67 +++++++++++++++++++++---------------- 3 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp index e3e3b07d71..701035fc32 100644 --- a/src/widgets/widgets/qmenu.cpp +++ b/src/widgets/widgets/qmenu.cpp @@ -2322,7 +2322,7 @@ void QMenu::popup(const QPoint &p, QAction *atAction) d->popup(p, atAction); } -void QMenuPrivate::popup(const QPoint &p, QAction *atAction) +void QMenuPrivate::popup(const QPoint &p, QAction *atAction, PositionFunction positionFunction) { Q_Q(QMenu); if (scroll) { // reset scroll state from last popup @@ -2388,6 +2388,10 @@ void QMenuPrivate::popup(const QPoint &p, QAction *atAction) const QSize menuSizeHint(q->sizeHint()); QSize size = menuSizeHint; + + if (positionFunction) + pos = positionFunction(menuSizeHint); + const int desktopFrame = q->style()->pixelMetric(QStyle::PM_MenuDesktopFrameWidth, nullptr, q); bool adjustToDesktop = !q->window()->testAttribute(Qt::WA_DontShowOnScreen); @@ -2642,14 +2646,14 @@ QAction *QMenu::exec(const QPoint &p, QAction *action) return d->exec(p, action); } -QAction *QMenuPrivate::exec(const QPoint &p, QAction *action) +QAction *QMenuPrivate::exec(const QPoint &p, QAction *action, PositionFunction positionFunction) { Q_Q(QMenu); q->ensurePolished(); q->createWinId(); QEventLoop evtLoop; eventLoop = &evtLoop; - popup(p, action); + popup(p, action, positionFunction); QPointer guard = q; (void) evtLoop.exec(); diff --git a/src/widgets/widgets/qmenu_p.h b/src/widgets/widgets/qmenu_p.h index 7d2d0ab487..02fe9f31a1 100644 --- a/src/widgets/widgets/qmenu_p.h +++ b/src/widgets/widgets/qmenu_p.h @@ -64,6 +64,8 @@ #include +#include + QT_REQUIRE_CONFIG(menu); QT_BEGIN_NAMESPACE @@ -302,6 +304,8 @@ class QMenuPrivate : public QWidgetPrivate { Q_DECLARE_PUBLIC(QMenu) public: + using PositionFunction = std::function; + QMenuPrivate() : itemsDirty(false), hasCheckableItems(false), @@ -351,8 +355,8 @@ public: QRect popupGeometry(int screen) const; bool useFullScreenForPopup() const; int getLastVisibleAction() const; - void popup(const QPoint &p, QAction *atAction); - QAction *exec(const QPoint &p, QAction *action); + void popup(const QPoint &p, QAction *atAction, PositionFunction positionFunction = {}); + QAction *exec(const QPoint &p, QAction *action, PositionFunction positionFunction = {}); //selection static QMenu *mouseDown; diff --git a/src/widgets/widgets/qtoolbutton.cpp b/src/widgets/widgets/qtoolbutton.cpp index 263fe5d8ce..314c6ab40c 100644 --- a/src/widgets/widgets/qtoolbutton.cpp +++ b/src/widgets/widgets/qtoolbutton.cpp @@ -723,38 +723,12 @@ void QToolButtonPrivate::_q_buttonReleased() popupTimer.stop(); } -void QToolButtonPrivate::popupTimerDone() +static QPoint positionMenu(const QToolButton *q, bool horizontal, + const QSize &sh) { - Q_Q(QToolButton); - popupTimer.stop(); - if (!menuButtonDown && !down) - return; - - menuButtonDown = true; - QPointer actualMenu; - bool mustDeleteActualMenu = false; - if(menuAction) { - actualMenu = menuAction->menu(); - } else if (defaultAction && defaultAction->menu()) { - actualMenu = defaultAction->menu(); - } else { - actualMenu = new QMenu(q); - mustDeleteActualMenu = true; - for(int i = 0; i < actions.size(); i++) - actualMenu->addAction(actions.at(i)); - } - repeat = q->autoRepeat(); - q->setAutoRepeat(false); - bool horizontal = true; -#if QT_CONFIG(toolbar) - QToolBar *tb = qobject_cast(parent); - if (tb && tb->orientation() == Qt::Vertical) - horizontal = false; -#endif QPoint p; const QRect rect = q->rect(); // Find screen via point in case of QGraphicsProxyWidget. QRect screen = QDesktopWidgetPrivate::availableGeometry(q->mapToGlobal(rect.center())); - QSize sh = ((QToolButton*)(QMenu*)actualMenu)->receivers(SIGNAL(aboutToShow()))? QSize() : actualMenu->sizeHint(); if (horizontal) { if (q->isRightToLeft()) { if (q->mapToGlobal(QPoint(0, rect.bottom())).y() + sh.height() <= screen.bottom()) { @@ -788,6 +762,37 @@ void QToolButtonPrivate::popupTimerDone() } p.rx() = qMax(screen.left(), qMin(p.x(), screen.right() - sh.width())); p.ry() += 1; + return p; +} + +void QToolButtonPrivate::popupTimerDone() +{ + Q_Q(QToolButton); + popupTimer.stop(); + if (!menuButtonDown && !down) + return; + + menuButtonDown = true; + QPointer actualMenu; + bool mustDeleteActualMenu = false; + if (menuAction) { + actualMenu = menuAction->menu(); + } else if (defaultAction && defaultAction->menu()) { + actualMenu = defaultAction->menu(); + } else { + actualMenu = new QMenu(q); + mustDeleteActualMenu = true; + for (int i = 0; i < actions.size(); i++) + actualMenu->addAction(actions.at(i)); + } + repeat = q->autoRepeat(); + q->setAutoRepeat(false); + bool horizontal = true; +#if QT_CONFIG(toolbar) + QToolBar *tb = qobject_cast(parent); + if (tb && tb->orientation() == Qt::Vertical) + horizontal = false; +#endif QPointer that = q; actualMenu->setNoReplayFor(q); if (!mustDeleteActualMenu) //only if action are not in this widget @@ -796,7 +801,11 @@ void QToolButtonPrivate::popupTimerDone() actualMenu->d_func()->causedPopup.widget = q; actualMenu->d_func()->causedPopup.action = defaultAction; actionsCopy = q->actions(); //(the list of action may be modified in slots) - actualMenu->exec(p); + + // QTBUG-78966, Delay positioning until after aboutToShow(). + auto positionFunction = [q, horizontal](const QSize &sizeHint) { + return positionMenu(q, horizontal, sizeHint); }; + actualMenu->d_func()->exec({}, nullptr, positionFunction); if (!that) return; -- cgit v1.2.3