Skip to content

Commit

Permalink
QWidgetWindow: send QContextMenuEvent even after accepted mouse press
Browse files Browse the repository at this point in the history
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 84a5f50

[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
Pick-to: 6.9
Change-Id: I454813dab4c387112f161fc28a0ee94570013afa
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
  • Loading branch information
ec1oud authored and kbroulik committed Dec 10, 2024
1 parent f3d5bdf commit 357c64a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
28 changes: 17 additions & 11 deletions src/gui/kernel/qwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2618,14 +2618,16 @@ bool QWindow::event(QEvent *ev)
case QEvent::MouseButtonPress: {
auto *me = static_cast<QMouseEvent*>(ev);
mousePressEvent(me);
d->maybeSynthesizeContextMenuEvent(me);
if (!ev->isAccepted())
d->maybeSynthesizeContextMenuEvent(me);
break;
}

case QEvent::MouseButtonRelease: {
auto *me = static_cast<QMouseEvent*>(ev);
mouseReleaseEvent(me);
d->maybeSynthesizeContextMenuEvent(me);
if (!ev->isAccepted())
d->maybeSynthesizeContextMenuEvent(me);
break;
}

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion src/widgets/kernel/qwidgetwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 357c64a

Please sign in to comment.