From f0758fb1130b64e5acea63bf86e1991b569ea1e5 Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen Date: Tue, 10 Sep 2024 15:28:49 +0200 Subject: [PATCH] QMenu: don't trigger a menu item if the mouse didn't move If you right-click to open a context menu, followed by another click to close it again, without moving the mouse, it can happen that the menu item under the mouse will trigger. This is especially prone to happen if Qt has a large scale factor and the number of menu items is high, since then the menu has to move a bit up to make room for as menu items as possible on the screen. This patch will reuse the already existing hasMouseMoved() function to avoid triggering a menu item if the mouse hasn't moved. This will make sure that the user won't trigger a menu item by accident when instead trying to close it. Fixes: QTBUG-128359 Pick-to: 6.7 6.5 6.2 Change-Id: Ie4d73ee2031d1159dd6c003124869004b4928d18 Reviewed-by: Shawn Rutledge (cherry picked from commit dc1d3abd7e852829b9bc81c1ba9565f40147402d) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/widgets/qmenu.cpp | 17 +++++++--- .../auto/widgets/widgets/qmenu/tst_qmenu.cpp | 32 +++++++++++++++++++ .../widgets/qtoolbutton/tst_qtoolbutton.cpp | 5 +++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp index db00f8a6507..0ce07aee40a 100644 --- a/src/widgets/widgets/qmenu.cpp +++ b/src/widgets/widgets/qmenu.cpp @@ -2459,7 +2459,7 @@ void QMenuPrivate::popup(const QPoint &p, QAction *atAction, PositionFunction po } } - QPoint mouse = QCursor::pos(); + const QPoint mouse = QGuiApplicationPrivate::lastCursorPosition.toPoint(); mousePopupPos = mouse; const bool snapToMouse = !causedPopup.widget && (QRect(p.x() - 3, p.y() - 3, 6, 6).contains(mouse)); @@ -2892,7 +2892,9 @@ void QMenu::mousePressEvent(QMouseEvent *e) // and mouse clicks on second screen, e->pos() is QPoint(0,0) and the menu doesn't hide. This trick makes // possible to hide the menu when mouse clicks on another screen (e->screenPos() returns correct value). // Only when mouse clicks in QPoint(0,0) on second screen, the menu doesn't hide. - if ((e->position().toPoint().isNull() && !e->globalPosition().isNull()) || !rect().contains(e->position().toPoint())) { + if ((e->position().toPoint().isNull() && !e->globalPosition().isNull()) + || !rect().contains(e->position().toPoint()) + || !d->hasMouseMoved(e->globalPosition().toPoint())) { if (d->noReplayFor && QRect(d->noReplayFor->mapToGlobal(QPoint()), d->noReplayFor->size()).contains(e->globalPosition().toPoint())) setAttribute(Qt::WA_NoMouseReplay); @@ -2923,8 +2925,15 @@ void QMenu::mouseReleaseEvent(QMouseEvent *e) QMenuPrivate::mouseDown = nullptr; d->setSyncAction(); - QAction *action = d->actionAt(e->position().toPoint()); + if (!d->hasMouseMoved(e->globalPosition().toPoint())) { + // We don't want to trigger a menu item if the mouse hasn't moved + // since the popup was opened. Instead we want to close the menu. + d->hideUpToMenuBar(); + return; + } + + QAction *action = d->actionAt(e->position().toPoint()); if (action && action == d->currentAction) { if (!action->menu()) { #if defined(Q_OS_WIN) @@ -2933,7 +2942,7 @@ void QMenu::mouseReleaseEvent(QMouseEvent *e) #endif d->activateAction(action, QAction::Trigger); } - } else if ((!action || action->isEnabled()) && d->hasMouseMoved(e->globalPosition().toPoint())) { + } else if (!action || action->isEnabled()) { d->hideUpToMenuBar(); } } diff --git a/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp b/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp index 18808cce160..aa6823ed690 100644 --- a/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp +++ b/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp @@ -116,6 +116,7 @@ private slots: void deleteWhenTriggered(); void nestedTearOffDetached(); + void closeMenuOnClickIfMouseHasntMoved(); protected slots: void onActivated(QAction*); @@ -2113,5 +2114,36 @@ void tst_QMenu::nestedTearOffDetached() QTest::mouseClick(subSubMenu, Qt::LeftButton, {}, QPoint(subSubMenu->width() / 2, tearOffOffset)); } +/*! + Test that a menu will close if you do a mouse click on top of + it without having moved the mouse. + (QTBUG-128359). +*/ +void tst_QMenu::closeMenuOnClickIfMouseHasntMoved() +{ + QWidget w; + w.resize(100, 100); + w.show(); + QVERIFY(QTest::qWaitForWindowExposed(&w)); + + QMenu contextMenu; + for (int i = 0; i < 5; ++i) { + QAction *action = contextMenu.addAction(QStringLiteral("action")); + connect(action, &QAction::triggered, []{ QFAIL("No menu item should trigger"); }); + } + + const QPoint pos = w.rect().center(); + const QPoint globalPos = w.mapToGlobal(pos); + // Move the mouse inside the window + QTest::mouseMove(&w, pos); + // Move the menu a bit up, so that a menu item falls underneath the + // mouse (similar to the code attached to the bug report: QTBUG-128359). + contextMenu.popup(globalPos - QPoint(0, 20)); + QVERIFY(QTest::qWaitForWindowExposed(&contextMenu)); + // Do a mouse click without having moved the cursor. This + // should close the menu, even if it's underneath the mouse. + QTest::mouseClick(&contextMenu, Qt::RightButton, {}, contextMenu.mapFromGlobal(pos)); +} + QTEST_MAIN(tst_QMenu) #include "tst_qmenu.moc" diff --git a/tests/auto/widgets/widgets/qtoolbutton/tst_qtoolbutton.cpp b/tests/auto/widgets/widgets/qtoolbutton/tst_qtoolbutton.cpp index b77a9c0eec8..81a8fb0efca 100644 --- a/tests/auto/widgets/widgets/qtoolbutton/tst_qtoolbutton.cpp +++ b/tests/auto/widgets/widgets/qtoolbutton/tst_qtoolbutton.cpp @@ -130,6 +130,11 @@ void tst_QToolButton::triggered() m_menu = menu.data(); + // QMenu uses QGuiApplicationPrivate::lastCursorPosition to detect pointer + // movement. And GuiApplication needs at least one mouse move to properly + // initialize it. So we send a mouse move now, before we open the menu. + QTest::mouseMove(mainWidget.windowHandle(), mainWidget.mapFromGlobal(QPoint(0, 0))); + QTimer *timer = new QTimer(this); timer->setInterval(50); connect(timer, SIGNAL(timeout()), this, SLOT(sendMouseClick()));