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 <volker.hilsheimer@qt.io>

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<QObject> 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 <qpa/qplatformmenu.h>
 
+#include <functional>
+
 QT_REQUIRE_CONFIG(menu);
 
 QT_BEGIN_NAMESPACE
@@ -302,6 +304,8 @@ class QMenuPrivate : public QWidgetPrivate
 {
     Q_DECLARE_PUBLIC(QMenu)
 public:
+    using PositionFunction = std::function<QPoint(const QSize &)>;
+
     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<QMenu> 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<QToolBar*>(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<QMenu> 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<QToolBar*>(parent);
+    if (tb && tb->orientation() == Qt::Vertical)
+        horizontal = false;
+#endif
     QPointer<QToolButton> 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;