Skip to content

Commit

Permalink
Quick Popup: ignore unhandled events
Browse files Browse the repository at this point in the history
QQuickPopups event handling code deals with two concepts: the handling
of events by the popup (accepting the event and becoming the grabber for
the relevant device in case of press events), and the blocking of events
to other items. For a plain popup that's ok, but events that aren't
blocked by the popup must be ignored by it to that they propagate
correctly.

QQuickDrawer, which subclasses QQuickPopup and overrides the blocking
logik (via QQuickDrawerPrivate::blockInput), conflates those two
concepts, which is not ok as QQuickDrawer is interactive and may accept
an event even if the event is not modally blocked. Press events that go
to the content item of the drawer are never blocked, but must get
accepted so that the drawer receives the following mouse moves.

Fixes: QTBUG-93649
Pick-to: 6.3 6.2
Change-Id: I254ccd843dc0250d334abb9b1062e7feffb86beb
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
  • Loading branch information
vohi committed May 18, 2022
1 parent 1818d3d commit f0b278c
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 7 deletions.
9 changes: 8 additions & 1 deletion src/quicktemplates2/qquickdrawer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,13 @@ bool QQuickDrawerPrivate::grabTouch(QQuickItem *item, QTouchEvent *event)

static const qreal openCloseVelocityThreshold = 300;

// Overrides QQuickPopupPrivate::blockInput, which is called by
// QQuickPopupPrivate::handlePress/Move/Release, which we call in our own
// handlePress/Move/Release overrides.
// This implementation conflates two things: should the event going to the item get
// modally blocked by us? Or should we accept the event and become the grabber?
// Those are two fundamentally different questions for the drawer as a (usually)
// interactive control.
bool QQuickDrawerPrivate::blockInput(QQuickItem *item, const QPointF &point) const
{
Q_Q(const QQuickDrawer);
Expand Down Expand Up @@ -460,7 +467,7 @@ bool QQuickDrawerPrivate::handlePress(QQuickItem *item, const QPointF &point, ul
velocityCalculator.startMeasuring(point, timestamp);

if (!QQuickPopupPrivate::handlePress(item, point, timestamp))
return false;
return interactive && popupItem == item;

return true;
}
Expand Down
9 changes: 3 additions & 6 deletions src/quicktemplates2/qquickpopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2550,22 +2550,19 @@ void QQuickPopup::keyReleaseEvent(QKeyEvent *event)
void QQuickPopup::mousePressEvent(QMouseEvent *event)
{
Q_D(QQuickPopup);
d->handleMouseEvent(d->popupItem, event);
event->accept();
event->setAccepted(d->handleMouseEvent(d->popupItem, event));
}

void QQuickPopup::mouseMoveEvent(QMouseEvent *event)
{
Q_D(QQuickPopup);
d->handleMouseEvent(d->popupItem, event);
event->accept();
event->setAccepted(d->handleMouseEvent(d->popupItem, event));
}

void QQuickPopup::mouseReleaseEvent(QMouseEvent *event)
{
Q_D(QQuickPopup);
d->handleMouseEvent(d->popupItem, event);
event->accept();
event->setAccepted(d->handleMouseEvent(d->popupItem, event));
}

void QQuickPopup::mouseDoubleClickEvent(QMouseEvent *event)
Expand Down
76 changes: 76 additions & 0 deletions tests/auto/quickcontrols2/controls/data/tst_popup.qml
Original file line number Diff line number Diff line change
Expand Up @@ -1441,4 +1441,80 @@ TestCase {
compare(shortcutActivatedSpy.count, 2)
tryCompare(control, "visible", false)
}

Component {
id: mousePropagationComponent
ApplicationWindow {
id: window
width: 360
height: 360
visible: true

property alias popup: popup
property alias popupTitle: popupTitle

Popup {
id: popup
width: 200
height: 200

background: Rectangle {
color: "#505050"
Rectangle {
id: popupTitle
width: parent.width
height: 30
color: "blue"

property point pressedPosition: Qt.point(0, 0)

MouseArea {
enabled: true
propagateComposedEvents: true
anchors.fill: parent
onPressed: (mouse) => {
popupTitle.pressedPosition = Qt.point(mouse.x, mouse.y)
}
onPositionChanged: (mouse) => {
popup.x += mouse.x - popupTitle.pressedPosition.x
popup.y += mouse.y - popupTitle.pressedPosition.y
}
onReleased: (mouse) => {
popupTitle.pressedPosition = Qt.point(0, 0)
}
}
}
}

Component.onCompleted: {
x = parent.width / 2 - width / 2
y = parent.height / 2 - height / 2
}
}
}
}

function test_mousePropagation() {
// Tests that Popup ignores mouse events that it doesn't handle itself
// so that they propagate correctly.
let window = createTemporaryObject(mousePropagationComponent, testCase)
window.requestActivate()
tryCompare(window, "active", true)

let popup = window.popup
popup.open()
let title = window.popupTitle
verify(title)

let pressPoint = Qt.point(title.width / 2, title.height / 2)
let oldPos = Qt.point(popup.x, popup.y)
mousePress(title, pressPoint.x, pressPoint.y)
fuzzyCompare(title.pressedPosition.x, pressPoint.x, 1)
fuzzyCompare(title.pressedPosition.y, pressPoint.y, 1)
mouseMove(title, pressPoint.x + 5, pressPoint.y + 5)
fuzzyCompare(popup.x, oldPos.x + 5, 1)
fuzzyCompare(popup.y, oldPos.y + 5, 1)
mouseRelease(title, pressPoint.x, pressPoint.y)
compare(title.pressedPosition, Qt.point(0, 0))
}
}

0 comments on commit f0b278c

Please sign in to comment.