Deliver QContextMenuEvent via ExtraData.contextMenu, not event filter

Event filtering can be problematic. At least this was a more reasonable
use case for a filter than some others in Controls; still, we already
have a switch case in QQuickItem::event(), and plan to add a virtual
QQuickItem::contextMenuEvent() in Qt 7, so we might as well mock it up
now as QQuickItemPrivate::contextMenuEvent(). Dispatching only the
QContextMenuEvents ought to be cheaper than filtering all events.
On the downside, ExtraData gets a little bigger.

We add QQuickItemPrivate::setContextMenu() because
QQuickItem::ExtraData is not exported; and to enable categorized
logging when one menu is replaced with another, this setter returns
the previously-known menu, if any.

Pick-to: 6.9
Change-Id: I9f2553fb579409becf797046dcc473260320c6a5
Reviewed-by: Mitch Curtis <[email protected]>

src/quick/items/qquickitem.cpp

@@ -5802,6 +5802,18 @@ bool QQuickItemPrivate::handlePointerEvent(QPointerEvent *event, bool avoidGrabb
     return delivered;
 }
 
+#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0)
+bool QQuickItemPrivate::handleContextMenuEvent(QContextMenuEvent *event)
+#else
+bool QQuickItem::contextMenuEvent(QContextMenuEvent *event)
+#endif
+{
+    if (extra.isAllocated() && extra->contextMenu)
+        return extra->contextMenu->event(event);
+    event->ignore();
+    return false;
+}
+
 /*!
     Called when \a change occurs for this item.
 
@@ -9186,7 +9198,7 @@ bool QQuickItem::event(QEvent *ev)
         break;
     case QEvent::ContextMenu:
         // ### Qt 7: add virtual contextMenuEvent (and to QWindow?)
-        ev->ignore();
+        d->handleContextMenuEvent(static_cast<QContextMenuEvent*>(ev));
         break;
     default:
         return QObject::event(ev);
@@ -9492,6 +9504,17 @@ void QQuickItemPrivate::removePointerHandler(QQuickPointerHandler *h)
         extra.value().acceptedMouseButtons = extra.value().acceptedMouseButtonsWithoutHandlers;
 }
 
+/*! \internal
+    Replaces any existing context menu with the given \a menu,
+    and returns the one that was already set before, or \c nullptr.
+*/
+QObject *QQuickItemPrivate::setContextMenu(QObject *menu)
+{
+    QObject *ret = (extra.isAllocated() ? extra->contextMenu : nullptr);
+    extra.value().contextMenu = menu;
+    return ret;
+}
+
 #if QT_CONFIG(quick_shadereffect)
 QQuickItemLayer::QQuickItemLayer(QQuickItem *item)
     : m_item(item)
@@ -10053,7 +10076,7 @@ QQuickItemPrivate::ExtraData::ExtraData()
 : z(0), scale(1), rotation(0), opacity(1),
   contents(nullptr), screenAttached(nullptr), layoutDirectionAttached(nullptr),
   enterKeyAttached(nullptr),
-  keyHandler(nullptr),
+  keyHandler(nullptr), contextMenu(nullptr),
 #if QT_CONFIG(quick_shadereffect)
   layer(nullptr),
 #endif

src/quick/items/qquickitem.h

@@ -463,6 +463,9 @@ public Q_SLOTS:
     virtual void dropEvent(QDropEvent *);
 #endif
     virtual bool childMouseEventFilter(QQuickItem *, QEvent *);
+#if QT_VERSION >= QT_VERSION_CHECK(7, 0, 0)
+    virtual bool contextMenuEvent(QContextMenuEvent *event);
+#endif
 
     virtual QSGNode *updatePaintNode(QSGNode *, UpdatePaintNodeData *);
     virtual void releaseResources();

src/quick/items/qquickitem_p.h

@@ -272,6 +272,8 @@ class Q_QUICK_EXPORT QQuickItemPrivate
     virtual void addPointerHandler(QQuickPointerHandler *h);
     virtual void removePointerHandler(QQuickPointerHandler *h);
 
+    QObject *setContextMenu(QObject *menu);
+
     // data property
     static void data_append(QQmlListProperty<QObject> *, QObject *);
     static qsizetype data_count(QQmlListProperty<QObject> *);
@@ -430,6 +432,7 @@ class Q_QUICK_EXPORT QQuickItemPrivate
         QQuickEnterKeyAttached *enterKeyAttached;
         QQuickItemKeyFilter *keyHandler;
         QVector<QQuickPointerHandler *> pointerHandlers;
+        QObject *contextMenu;
 #if QT_CONFIG(quick_shadereffect)
         mutable QQuickItemLayer *layer;
 #endif
@@ -710,6 +713,9 @@ class Q_QUICK_EXPORT QQuickItemPrivate
 
     bool anyPointerHandlerWants(const QPointerEvent *event, const QEventPoint &point) const;
     virtual bool handlePointerEvent(QPointerEvent *, bool avoidGrabbers = false);
+#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0)
+    virtual bool handleContextMenuEvent(QContextMenuEvent *event);
+#endif
 
     virtual void setVisible(bool visible);
 

src/quicktemplates/qquickcontextmenu.cpp

@@ -8,6 +8,7 @@
 #include <QtQml/qqmlcomponent.h>
 #include <QtQml/qqmlinfo.h>
 #include <QtQuick/qquickwindow.h>
+#include <QtQuick/private/qquickitem_p.h>
 #include <QtQuickTemplates2/private/qquickmenu_p.h>
 
 QT_BEGIN_NAMESPACE
@@ -48,8 +49,14 @@ QQuickContextMenu::QQuickContextMenu(QObject *parent)
     : QObject(*(new QQuickContextMenuPrivate), parent)
 {
     Q_ASSERT(parent);
-    if (!parent->isQuickItemType())
+    if (parent->isQuickItemType()) {
+        auto *itemPriv = QQuickItemPrivate::get(static_cast<QQuickItem *>(parent));
+        Q_ASSERT(itemPriv);
+        if (QObject *oldMenu = itemPriv->setContextMenu(this))
+            qCWarning(lcContextMenu) << this << "replaced" << oldMenu << "on" << parent;
+    } else {
         qmlWarning(parent) << "ContextMenu must be attached to an Item";
+    }
 }
 
 QQuickContextMenu *QQuickContextMenu::qmlAttachedProperties(QObject *object)
@@ -78,44 +85,35 @@ void QQuickContextMenu::setMenu(QQuickMenu *menu)
     if (menu == d->menu)
         return;
 
-    if (d->menu) {
-        auto *attacheeItem = qobject_cast<QQuickItem *>(parent());
-        qCDebug(lcContextMenu) << this << "is removing its event filter on attachee" << attacheeItem;
-        attacheeItem->removeEventFilter(this);
-    }
-
     d->menu = menu;
-
-    if (d->menu) {
-        auto *attacheeItem = qobject_cast<QQuickItem *>(parent());
-        qCDebug(lcContextMenu) << this << "is installing an event filter on attachee" << attacheeItem;
-        attacheeItem->installEventFilter(this);
-    }
-
     emit menuChanged();
 }
 
-bool QQuickContextMenu::eventFilter(QObject *object, QEvent *event)
+bool QQuickContextMenu::event(QEvent *event)
 {
     switch (event->type()) {
     case QEvent::ContextMenu: {
-        qCDebug(lcContextMenu) << this << "is handling filtered ContextMenu event" << event;
+        qCDebug(lcContextMenu) << this << "handling" << event << "on behalf of" << parent();
 
         auto *attacheeItem = qobject_cast<QQuickItem *>(parent());
         auto *menu = this->menu();
+        if (!menu) {
+            qCDebug(lcContextMenu) << this << "no menu instance";
+            return QObject::event(event);
+        }
         menu->setParentItem(attacheeItem);
 
         const auto *contextMenuEvent = static_cast<QContextMenuEvent *>(event);
         const QPoint posRelativeToParent(attacheeItem->mapFromScene(contextMenuEvent->pos()).toPoint());
-        qCDebug(lcContextMenu) << this << "is showing menu instance" << menu << "at" << posRelativeToParent;
+        qCDebug(lcContextMenu) << this << "showing" << menu << "at" << posRelativeToParent;
         menu->popup(posRelativeToParent);
         event->accept();
         return true;
     }
     default:
         break;
     }
-    return QObject::eventFilter(object, event);
+    return QObject::event(event);
 }
 
 QT_END_NAMESPACE

src/quicktemplates/qquickcontextmenu_p.h

@@ -44,10 +44,11 @@ class Q_QUICKTEMPLATES2_EXPORT QQuickContextMenu : public QObject
 Q_SIGNALS:
     void menuChanged();
 
+protected:
+    bool event(QEvent *) override;
+
 private:
     Q_DECLARE_PRIVATE(QQuickContextMenu)
-
-    bool eventFilter(QObject *object, QEvent *event) override;
 };
 
 QT_END_NAMESPACE