From f329dd4e16c87b0670db3e997fb51ca4e810014e Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 9 Dec 2024 22:16:37 +0100 Subject: [PATCH] QWidgetWindow: send QContextMenuEvent even after accepted mouse press It would be more consistent if we could rely on the accepted state of the original QMouseEvent to decide whether to follow up with a QContextMenuEvent; but not all Qt widgets call ignore() on unhandled mouse events, both in Qt and in external libraries and applications (including some from KDE). So we should at least wait until Qt 7 to make this a requirement. It seems sensible to move the check into QWindow::event() rather than trying to distinguish the window type in maybeSynthesizeContextMenuEvent(). We merely output a categorized log message to indicate when the legacy behavior is in effect. Amends 84a5f50c7766c99f62b22bb4388137e0aa8dd13d [ChangeLog][QtWidgets] If your QWidget subclass depends on receiving QContextMenuEvent, and also handles mouse events directly, we recommend that you call ignore() on unhandled mouse events (such as right-button events). In Qt 7, we plan to stop sending QContextMenuEvent if the triggering mouse event is accepted. Fixes: QTBUG-132066 Change-Id: I454813dab4c387112f161fc28a0ee94570013afa Reviewed-by: Volker Hilsheimer Reviewed-by: Mitch Curtis (cherry picked from commit 357c64a99607456133bfabf86d6b67162717cb29) Reviewed-by: Qt Cherry-pick Bot --- src/gui/kernel/qwindow.cpp | 28 +++++++++++++++++----------- src/widgets/kernel/qwidgetwindow.cpp | 8 +++++++- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp index 77e12564849..0095a29af6a 100644 --- a/src/gui/kernel/qwindow.cpp +++ b/src/gui/kernel/qwindow.cpp @@ -2618,14 +2618,16 @@ bool QWindow::event(QEvent *ev) case QEvent::MouseButtonPress: { auto *me = static_cast(ev); mousePressEvent(me); - d->maybeSynthesizeContextMenuEvent(me); + if (!ev->isAccepted()) + d->maybeSynthesizeContextMenuEvent(me); break; } case QEvent::MouseButtonRelease: { auto *me = static_cast(ev); mouseReleaseEvent(me); - d->maybeSynthesizeContextMenuEvent(me); + if (!ev->isAccepted()) + d->maybeSynthesizeContextMenuEvent(me); break; } @@ -2748,18 +2750,21 @@ bool QWindow::event(QEvent *ev) } /*! \internal - Synthesize and send a QContextMenuEvent if the given \a event is a suitable - mouse event (a right-button press or release, depending on - QStyleHints::contextMenuTrigger()) that was *not accepted* and *isn't* - exclusively grabbed. On most platforms, it's done on mouse release; on + Synthesize and send a QContextMenuEvent if the given \a event is a + suitable mouse event (a right-button press or release, depending on + QStyleHints::contextMenuTrigger()) that *isn't* exclusively grabbed. + On most platforms, it's done on mouse release; on Windows, it's done on press, because of the potential to support right-button clicks and drags to select or lasso items, and then still getting a context menu at the end of that gesture. (That is in conflict with supporting the press-drag-release gesture to select menu items on the context menus themselves. Context menus can be implemented that way by handling the separate press, move and release events.) Any time the - \a event was already handled in some way, it must be accepted, to avoid - synthesis of the QContextMenuEvent here. + \a event was already handled in some way, it *should* be accepted, to + indicate that it's not necessary to synthesize a QContextMenuEvent here. + However, there's enough legacy widget code that doesn't call ignore() + on unhandled mouse events, that in QWidgetWindow, we put off requiring + the event to be ignored. Hopefully we can begin requiring it in Qt 7. The QContextMenuEvent occurs at the scenePosition(). The position() was likely already "localized" during the previous delivery. @@ -2779,12 +2784,13 @@ bool QWindow::event(QEvent *ev) void QWindowPrivate::maybeSynthesizeContextMenuEvent(QMouseEvent *event) { #ifndef QT_NO_CONTEXTMENU - if (!event->isAccepted() && !event->exclusivePointGrabber() - && event->button() == Qt::RightButton + if (!event->exclusivePointGrabber() && event->button() == Qt::RightButton && event->type() == QGuiApplicationPrivate::contextMenuEventType()) { QContextMenuEvent e(QContextMenuEvent::Mouse, event->scenePosition().toPoint(), event->globalPosition().toPoint(), event->modifiers()); - qCDebug(lcPopup) << "synthesized QContextMenuEvent after ignored" << event->type() << ":" << &e; + qCDebug(lcPopup) << "synthesized after" + << (event->isAccepted() ? "ACCEPTED (legacy behavior)" : "ignored") + << event->type() << ":" << &e; QGuiApplication::sendEvent(q_func(), &e); } #endif diff --git a/src/widgets/kernel/qwidgetwindow.cpp b/src/widgets/kernel/qwidgetwindow.cpp index d337679fb2d..0565835e38b 100644 --- a/src/widgets/kernel/qwidgetwindow.cpp +++ b/src/widgets/kernel/qwidgetwindow.cpp @@ -666,7 +666,13 @@ void QWidgetWindow::handleMouseEvent(QMouseEvent *event) event->setAccepted(translated.isAccepted()); } - d->maybeSynthesizeContextMenuEvent(event); +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) + if ( +#else + if (event->isAccepted() && +#endif + (event->type() == QEvent::MouseButtonPress || event->type() == QEvent::MouseButtonRelease)) + d->maybeSynthesizeContextMenuEvent(event); } void QWidgetWindow::handleTouchEvent(QTouchEvent *event)