Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-93486

Clean up delivery logic of QContextMenuEvent

    XMLWordPrintable

    Details

    • Technical Risk:
      Normal

      Description

      It is platform specific when QContextMenuEvents are triggered, but the actual logic doesn't live in the platform plugins today. Instead, QPlatformTheme::themeHint reports for QPlatformTheme::ContextMenuOnMouseRelease which mouse button triggers the context menu event.

      On all platforms we synthesize the event ourselves depending on that trigger info, at the end of the already insanely complex QWidgetWindow::event:

      #ifndef QT_NO_CONTEXTMENU
          if (event->type() == contextMenuTrigger && event->button() == Qt::RightButton
              && m_widget->rect().contains(event->position().toPoint())) {
              QContextMenuEvent e(QContextMenuEvent::Mouse, mapped, event->globalPosition().toPoint(), event->modifiers());
              QGuiApplication::forwardEvent(receiver, &e, event);
          }
      #endif
      

      On Windows, we handle WM_CONTEXTMENU to generate a QContextMenuEvent which we deliver via handleContextMenuEvent. However, that message is only generated by the DefWindowProc for WM_RBUTTONUP, which we only call if Qt didn't handle the event at all, and it's practically impossible to achive that even for QWindow (reimplementing event, ignoring the event and returning false is not enough!).

      So even though we never get a WM_CONTEXTMENU-originating QContextMenuEvent with Mouse trigger, in QWidgetWindow and in QGuieApplicationPrivate, we then ignore mouse-triggered context menu events. This at any rate avoids that we get both the platform-triggered event, and the synthesized event:

      void QWidgetWindow::handleContextMenuEvent(QContextMenuEvent *e)
      {
          // We are only interested in keyboard originating context menu events here,
          // mouse originated context menu events for widgets are generated in mouse handling methods.
          if (e->reason() != QContextMenuEvent::Keyboard)
              return;
      
      void QGuiApplicationPrivate::processContextMenuEvent(QWindowSystemInterfacePrivate::ContextMenuEvent *e)
      {
          // Widgets do not care about mouse triggered context menu events. Also, do not forward event
          // to a window blocked by a modal window.
          if (!e->window || e->mouseTriggered || e->window->d_func()->blockedByModalWindow)
              return;
      [...]
      

      This is all very messy, and we should clean this up.

      Perhaps we could obsolete the theme hint and let each platform plugin decide when and how to synthesize QContextMenuEvent, and then deliver those events as usual to any QWindow (as of the fix for QTBUG-59988 in https://codereview.qt-project.org/c/qt/qtbase/+/347659, QWindow also synthesizes the event). No special logic would be needed there or in QWidgetWindow.

      Note: as of right now the mouse event triggering the context menu (either press or release) is always delivered before the context menu event, both for widgets and (with the change mentioned above) windows. This is consistent with how Windows does it - only generate the WM_CONTEXTMENU if the WM_RBUTTONUP is ignored - and generally not something we should change, even though it would perhaps be nicer to inverse the order.

        Attachments

        For Gerrit Dashboard: QTBUG-93486
        # Subject Branch Project Status CR V

          Activity

            People

            Assignee:
            srutledg Shawn Rutledge
            Reporter:
            vhilshei Volker Hilsheimer
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:

                Gerrit Reviews

                There are no open Gerrit changes