From 8d2f2cc7916a5e33dd5e8735675eb81e7a4de23f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Tue, 20 Aug 2024 18:08:53 +0200 Subject: [PATCH] Avoid dock widget separators stealing mouse events from expanded toolbar The way QMainWindow handles dock widget separators is a bit evil. They are real QWidgets, added as direct children of the QMainWindow, but painting and mouse interaction is handled by a hook into the QMainWindow events, via QMainWindowLayoutSeparatorHelper::windowEvent(), which is called at the start of QMainWindow::event(). For reasons unknown, we also raise() these separator widgets, which means they are always at the top of the child window stack of the main window. This is problematic for the situation when a QToolBar has an expanded overflow-menu, which is also raised(), but where we end up raising the separators on top of the toolbar again. As a result the toolbar gets leave events when we hover one of the separators, even if the toolbar menu is overlaid the dock area, resulting in the menu closing. We don't see this visually, as the separators are drawn via the hook mentioned above, which happens before QMainWindow draws any of its child widgets, like the toolbar. Fixing this requires us to selectively place the separators below any expanded toolbar each time we raise a separator, so that we respect the request of the toolbar to be on top, while still ensuring that the separators are above any of the dock widgets. Unfortunately the QMainWindowLayoutSeparatorHelper hook also processes mouse move and click events, resulting in the cursor changing shape when hovering the separator, even if the toolbar menu is over it. To fix this we amend the logic of finding a separator widget based on the mouse event position to also querying the QWidget childAt() function, and if we are over a tool bar, we bail out instead of looking for a separator on that position in the dock area layout. As the special separator logic in QMainWindow is conditioned on the style returning 1 for QStyle::PM_DockWidgetSeparatorExtent the test checks for this, and skips if another style is in use. So far only the macOS style is affected by this. See the original change in fba33a6bbc09bb278df75829d8d705863d40a6a3. Fixes: QTBUG-124733 Pick-to: 6.5 6.2 Change-Id: Ifc741336bd9725ef0fea9f0474e27b7481f3a183 Reviewed-by: Volker Hilsheimer (cherry picked from commit 3d99f90b50ab22ef5ab4433c57f9ee584a0a7cae) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/widgets/qdockarealayout.cpp | 33 ++++++++++- src/widgets/widgets/qmainwindowlayout.cpp | 8 +++ src/widgets/widgets/qmainwindowlayout_p.h | 34 ++++++++++- src/widgets/widgets/qtoolbarlayout_p.h | 2 +- .../widgets/qmainwindow/tst_qmainwindow.cpp | 58 +++++++++++++++++++ 5 files changed, 130 insertions(+), 5 deletions(-) diff --git a/src/widgets/widgets/qdockarealayout.cpp b/src/widgets/widgets/qdockarealayout.cpp index da0e9871715..32d0f9b345a 100644 --- a/src/widgets/widgets/qdockarealayout.cpp +++ b/src/widgets/widgets/qdockarealayout.cpp @@ -19,6 +19,11 @@ #include "qdockwidget_p.h" #include +#if QT_CONFIG(toolbar) +#include "qtoolbar.h" +#include "qtoolbarlayout_p.h" +#endif + #include #include @@ -2036,6 +2041,30 @@ bool QDockAreaLayoutInfo::restoreState(QDataStream &stream, QList } #if QT_CONFIG(tabbar) + +static void raiseSeparatorWidget(QWidget *separatorWidget) +{ + Q_ASSERT(separatorWidget); + +#if QT_CONFIG(toolbar) + // Raise the separator widget, but make sure it doesn't go above + // an expanded toolbar, as that would break mouse event hit testing. + Q_ASSERT(separatorWidget->parent()); + const auto toolBars = separatorWidget->parent()->findChildren(Qt::FindDirectChildrenOnly); + for (auto *toolBar : toolBars) { + if (auto *toolBarLayout = qobject_cast(toolBar->layout())) { + if (toolBarLayout->expanded) { + separatorWidget->stackUnder(toolBar); + return; + } + } + } +#endif + + separatorWidget->raise(); +} + + void QDockAreaLayoutInfo::updateSeparatorWidgets() const { if (tabbed) { @@ -2077,7 +2106,7 @@ void QDockAreaLayoutInfo::updateSeparatorWidgets() const j++; Q_ASSERT(sepWidget); - sepWidget->raise(); + raiseSeparatorWidget(sepWidget); QRect sepRect = separatorRect(i).adjusted(-2, -2, 2, 2); sepWidget->setGeometry(sepRect); @@ -3362,7 +3391,7 @@ void QDockAreaLayout::updateSeparatorWidgets() const j++; Q_ASSERT(sepWidget); - sepWidget->raise(); + raiseSeparatorWidget(sepWidget); QRect sepRect = separatorRect(i).adjusted(-2, -2, 2, 2); sepWidget->setGeometry(sepRect); diff --git a/src/widgets/widgets/qmainwindowlayout.cpp b/src/widgets/widgets/qmainwindowlayout.cpp index 34c18c8f886..ee854615e90 100644 --- a/src/widgets/widgets/qmainwindowlayout.cpp +++ b/src/widgets/widgets/qmainwindowlayout.cpp @@ -274,6 +274,14 @@ public: QDockAreaLayoutInfo *dockAreaLayoutInfo() { return &layoutState; } +#if QT_CONFIG(toolbar) + QToolBarAreaLayout *toolBarAreaLayout() + { + auto *mainWindow = static_cast(parentWidget()); + return qt_mainwindow_layout(mainWindow)->toolBarAreaLayout(); + } +#endif + bool nativeWindowDeco() const { return groupWindow()->hasNativeDecos(); diff --git a/src/widgets/widgets/qmainwindowlayout_p.h b/src/widgets/widgets/qmainwindowlayout_p.h index 55a27e4849d..f4211046e1c 100644 --- a/src/widgets/widgets/qmainwindowlayout_p.h +++ b/src/widgets/widgets/qmainwindowlayout_p.h @@ -43,6 +43,7 @@ struct QDockWidgetPrivate { #endif #if QT_CONFIG(toolbar) #include "qtoolbararealayout_p.h" +#include "qtoolbar.h" #endif #include @@ -92,6 +93,9 @@ public: bool endSeparatorMove(const QPoint &pos); bool windowEvent(QEvent *e); +private: + QList findSeparator(const QPoint &pos) const; + #endif // QT_CONFIG(dockwidget) }; @@ -142,7 +146,7 @@ void QMainWindowLayoutSeparatorHelper::adjustCursor(const QPoint &pos) w->unsetCursor(); } } else if (movingSeparator.isEmpty()) { // Don't change cursor when moving separator - QList pathToSeparator = layout()->dockAreaLayoutInfo()->findSeparator(pos); + QList pathToSeparator = findSeparator(pos); if (pathToSeparator != hoverSeparator) { if (!hoverSeparator.isEmpty()) @@ -279,10 +283,35 @@ bool QMainWindowLayoutSeparatorHelper::windowEvent(QEvent *event) return false; } +template +QList QMainWindowLayoutSeparatorHelper::findSeparator(const QPoint &pos) const +{ + Layout *layout = const_cast(this->layout()); +#if QT_CONFIG(toolbar) + QToolBarAreaLayout *toolBarAreaLayout = layout->toolBarAreaLayout(); + if (!toolBarAreaLayout->isEmpty()) { + // We might have a toolbar that is currently expanded, covering + // parts of the dock area, in which case we don't want the dock + // area layout to treat mouse events for the expanded toolbar as + // hitting a separator. + const QWidget *widget = layout->window(); + QWidget *childWidget = widget->childAt(pos); + while (childWidget && childWidget != widget) { + if (auto *toolBar = qobject_cast(childWidget)) { + if (!toolBarAreaLayout->indexOf(toolBar).isEmpty()) + return {}; + } + childWidget = childWidget->parentWidget(); + } + } +#endif + return layout->dockAreaLayoutInfo()->findSeparator(pos); +} + template bool QMainWindowLayoutSeparatorHelper::startSeparatorMove(const QPoint &pos) { - movingSeparator = layout()->dockAreaLayoutInfo()->findSeparator(pos); + movingSeparator = findSeparator(pos); if (movingSeparator.isEmpty()) return false; @@ -493,6 +522,7 @@ public: void removeToolBar(QToolBar *toolbar); void toggleToolBarsVisible(); void moveToolBar(QToolBar *toolbar, int pos); + QToolBarAreaLayout *toolBarAreaLayout() { return &layoutState.toolBarAreaLayout; } #endif // dock widgets diff --git a/src/widgets/widgets/qtoolbarlayout_p.h b/src/widgets/widgets/qtoolbarlayout_p.h index a7d3810d619..7915af1e44b 100644 --- a/src/widgets/widgets/qtoolbarlayout_p.h +++ b/src/widgets/widgets/qtoolbarlayout_p.h @@ -38,7 +38,7 @@ public: bool customWidget; }; -class QToolBarLayout : public QLayout +class Q_AUTOTEST_EXPORT QToolBarLayout : public QLayout { Q_OBJECT diff --git a/tests/auto/widgets/widgets/qmainwindow/tst_qmainwindow.cpp b/tests/auto/widgets/widgets/qmainwindow/tst_qmainwindow.cpp index 093af90d1cb..bf0f58587a4 100644 --- a/tests/auto/widgets/widgets/qmainwindow/tst_qmainwindow.cpp +++ b/tests/auto/widgets/widgets/qmainwindow/tst_qmainwindow.cpp @@ -22,6 +22,8 @@ #include #include #include +#include +#include #if QT_CONFIG(tabbar) #include @@ -133,6 +135,9 @@ private slots: #if QT_CONFIG(dockwidget) && QT_CONFIG(tabbar) void QTBUG52175_tabifiedDockWidgetActivated(); #endif +#ifdef QT_BUILD_INTERNAL + void expandedToolBarHitTesting(); +#endif }; @@ -2255,5 +2260,58 @@ void tst_QMainWindow::QTBUG52175_tabifiedDockWidgetActivated() } #endif +#ifdef QT_BUILD_INTERNAL +void tst_QMainWindow::expandedToolBarHitTesting() +{ + QMainWindow mainWindow; + if (mainWindow.style()->pixelMetric( + QStyle::PM_DockWidgetSeparatorExtent, nullptr, &mainWindow) != 1) { + QSKIP("Style does not trigger the use of qt_qmainwindow_extended_splitter"); + } + + mainWindow.setAnimated(false); + + auto *dockWidget = new QDockWidget(&mainWindow); + dockWidget->setWidget(new QWidget(dockWidget)); + mainWindow.addDockWidget(Qt::RightDockWidgetArea, dockWidget); + + auto *toolBar = new QToolBar(&mainWindow); + for (int i = 0; i < 10; ++i) + toolBar->addWidget(new QLabel(QString("Label %1").arg(i))); + mainWindow.addToolBar(toolBar); + + auto *centralWidget = new QWidget(&mainWindow); + centralWidget->setMinimumSize(QSize(100, 100)); + mainWindow.setCentralWidget(centralWidget); + + mainWindow.resize(centralWidget->minimumSize()); + mainWindow.show(); + QVERIFY(QTest::qWaitForWindowActive(&mainWindow)); + + auto *toolBarExtension = toolBar->findChild(); + QVERIFY(toolBarExtension); + QPoint buttonCenter = toolBarExtension->parentWidget()->mapTo(&mainWindow, toolBarExtension->geometry().center()); + QTest::mouseMove(mainWindow.windowHandle(), buttonCenter); + QTest::mouseClick(mainWindow.windowHandle(), Qt::LeftButton, Qt::NoModifier, buttonCenter); + + auto *toolBarLayout = static_cast(toolBar->layout()); + QVERIFY(toolBarLayout); + QTRY_COMPARE(toolBarLayout->expanded, true); + + auto *splitter = mainWindow.findChild("qt_qmainwindow_extended_splitter"); + QVERIFY(splitter); + QCOMPARE(splitter->parentWidget(), &mainWindow); + + // Moving the mouse over the splitter when it's covered by the toolbar + // extension area should not trigger a closing of the extension area. + QTest::mouseMove(mainWindow.windowHandle(), splitter->geometry().center()); + QCOMPARE(toolBarLayout->expanded, true); + + // Nor should it result in a split cursor shape, indicating we can move + // the splitter. + QCOMPARE(mainWindow.cursor().shape(), Qt::ArrowCursor); +} +#endif // QT_BUILD_INTERNAL + QTEST_MAIN(tst_QMainWindow) #include "tst_qmainwindow.moc"