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>
This commit is contained in:
Friedemann Kleint 2019-12-05 09:33:50 +01:00
parent ef14e775de
commit a78d667431
3 changed files with 51 additions and 34 deletions

View File

@ -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();

View File

@ -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;

View File

@ -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;