QMenu: don't show an empty menu
If a menu contains no or only invisible actions, then its sizeHint might become invalid (depending on style), resulting in an invalid geometry. In that case, return early from popup(). Otherwise we try to show popup window with an invalid size, which results in unpredictable behavior, depending on the windowing system. To minimize possible fallout from this change, don't return early from QWidget::show (or QWindow::setVisible), but fix this locally in QMenu. The QMenuBar test used empty menus to confirm that hovering over other menu bar entries would highlight the menu bar entry under the mojuse, and close the currently open menu. This can be tested better and more reliably with menus that are not empty, which is (probably) also going to fix the test on wayland. Fixes: QTBUG-129108 Change-Id: Icc52528e89baefea04b3b27e6f02674bf74162b2 Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io> Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io> (cherry picked from commit 353ce5344fbde5a6cecbdd2c131e1cf0f4b7f383) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
parent
ca65f94003
commit
686c4835c1
@ -2459,6 +2459,13 @@ void QMenuPrivate::popup(const QPoint &p, QAction *atAction, PositionFunction po
|
||||
}
|
||||
}
|
||||
|
||||
// do nothing if we don't have a valid size, e.g. when all actions are invisible
|
||||
if (!size.isValid()) {
|
||||
eventLoop = nullptr;
|
||||
syncAction = nullptr;
|
||||
return;
|
||||
}
|
||||
|
||||
const QPoint mouse = QGuiApplicationPrivate::lastCursorPosition.toPoint();
|
||||
mousePopupPos = mouse;
|
||||
const bool snapToMouse = !causedPopup.widget && (QRect(p.x() - 3, p.y() - 3, 6, 6).contains(mouse));
|
||||
@ -2505,6 +2512,7 @@ void QMenuPrivate::popup(const QPoint &p, QAction *atAction, PositionFunction po
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const int subMenuOffset = q->style()->pixelMetric(QStyle::PM_SubMenuOverlap, nullptr, q);
|
||||
QMenu *caused = qobject_cast<QMenu*>(causedPopup.widget);
|
||||
if (caused && caused->geometry().width() + menuSizeHint.width() + subMenuOffset < screen.width()) {
|
||||
@ -2667,7 +2675,8 @@ QAction *QMenuPrivate::exec(const QPoint &p, QAction *action, PositionFunction p
|
||||
popup(p, action, positionFunction);
|
||||
|
||||
QPointer<QObject> guard = q;
|
||||
(void) evtLoop.exec();
|
||||
if (eventLoop) // popup might have reset if there was nothing to show
|
||||
(void)eventLoop->exec();
|
||||
if (guard.isNull())
|
||||
return nullptr;
|
||||
|
||||
|
@ -118,6 +118,8 @@ private slots:
|
||||
void nestedTearOffDetached();
|
||||
void closeMenuOnClickIfMouseHasntMoved();
|
||||
|
||||
void invisibleActions();
|
||||
|
||||
protected slots:
|
||||
void onActivated(QAction*);
|
||||
void onHighlighted(QAction*);
|
||||
@ -2145,5 +2147,37 @@ void tst_QMenu::closeMenuOnClickIfMouseHasntMoved()
|
||||
QTest::mouseClick(&contextMenu, Qt::RightButton, {}, contextMenu.mapFromGlobal(pos));
|
||||
}
|
||||
|
||||
void tst_QMenu::invisibleActions()
|
||||
{
|
||||
QWidget window;
|
||||
window.resize(100, 100);
|
||||
window.show();
|
||||
|
||||
const QPoint globalPos = window.mapToGlobal(window.rect().center());
|
||||
|
||||
QVERIFY(QTest::qWaitForWindowExposed(&window));
|
||||
|
||||
QMenu contextMenu;
|
||||
QList<QAction *> actions;
|
||||
for (int i = 0; i < 5; ++i)
|
||||
actions << contextMenu.addAction("action");
|
||||
QVERIFY(contextMenu.sizeHint().isValid());
|
||||
|
||||
contextMenu.popup(globalPos);
|
||||
QVERIFY(contextMenu.isVisible());
|
||||
|
||||
contextMenu.close();
|
||||
|
||||
for (const auto &action : actions)
|
||||
action->setVisible(false);
|
||||
|
||||
contextMenu.popup(globalPos);
|
||||
QCOMPARE(contextMenu.isVisible(), contextMenu.sizeHint().isValid());
|
||||
|
||||
// if it wasn't shown previously, then exec() shouldn't do anything either
|
||||
if (!contextMenu.isVisible())
|
||||
QVERIFY(!contextMenu.exec());
|
||||
}
|
||||
|
||||
QTEST_MAIN(tst_QMenu)
|
||||
#include "tst_qmenu.moc"
|
||||
|
@ -1090,15 +1090,14 @@ void tst_QMenuBar::task256322_highlight()
|
||||
if (!QGuiApplication::platformName().compare(QLatin1String("minimal"), Qt::CaseInsensitive))
|
||||
QSKIP("Highlighting does not work correctly for minimal platform");
|
||||
|
||||
if (QGuiApplication::platformName().startsWith(QLatin1String("wayland"), Qt::CaseInsensitive))
|
||||
QSKIP("Wayland: This fails. Figure out why.");
|
||||
|
||||
QMainWindow win;
|
||||
win.menuBar()->setNativeMenuBar(false); //we can't check the geometry of native menubars
|
||||
QMenu menu;
|
||||
menu.addAction("New");
|
||||
QAction *file = win.menuBar()->addMenu(&menu);
|
||||
file->setText("file");
|
||||
QMenu menu2;
|
||||
menu2.addAction("Open");
|
||||
QAction *file2 = win.menuBar()->addMenu(&menu2);
|
||||
file2->setText("file2");
|
||||
QAction *nothing = win.menuBar()->addAction("nothing");
|
||||
|
Loading…
x
Reference in New Issue
Block a user