mouse handling: fix issue when mixing QTest::mouse* APIs

... that become apparent after switching qtestlib to use enhanced mouse
event (a37785ec76). With the old code path,
where QGuiApplication was deducing event type it would deduce mouse release
event even when there wasn't one. The new code path doesn't do that, which
revealed an obscure problem when mixing QTest::mouse* APIs (where QWindow
overload goes through QWindowSystemInterface API and QWidget overload goes
through QApplication::notify() and sets mouse_buttons from there). What
happened in this specific test case "./tst_qtreeview selection statusTip" was:

// tst_QTreeView::selection sets mouse_buttons = Qt::LeftButton from QApplication::notify
QTest::mousePress(widget, Qt::LeftButton, ..)

// tst_QTreeView::statusTip
QTest::mouseMove(window, )

The old code path sees that position and state has changed, creates a fake
mouse event, which gets deduced as mouse release even if there wasn't one.
And by luck this happened to set mouse_buttons=Qt::NoButton. So when we use
mouse_buttons later to create QMouseEvent everything works as expected. With
the enhanced mouse we don't clear the pressed button from mouse_buttons (set
in tst_QTreeView::selection) as this is done only from press/release events,
then pass it to QMouseEvent and later because of that QApplicationPrivate::
pickMouseReceiver() returns nullptr.

The fix here is to use e->buttons when constructing QMouseEvent, instead of
relying on mouse_buttons which gets changed from various places and has other
issues that can not be solved without invalidating the current documentation
of QGuiApplication::mouseButtons() (e.g QTBUG-33161). Tests and any Qt code
in general should avoid using the fragile QGuiApplication::mouseButtons() API.
This patch does not affect the old code path (it continues working as before)
and fixes the issue described above for the enhanced mouse API. The enhanced
mouse API actually is better in a way that it does not get affected by button
state from test functions that run earlier, as opposed to the old code path
where every subsequent test function uses mouse_buttons in whatever state it
was left by the test functions that run earlier.

Not relying on mouse_buttons when creating QMouseEvent helped also to discover
other logic error. This caused an in incorrect button state for a mouse move
event that is generated for a release event that simultaneously changes a mouse
position.

Task-number: QTBUG-64043
Change-Id: I6ad8e49d8437ab0858180c2d0d45694f3b3c2d60
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
This commit is contained in:
Gatis Paeglis 2017-10-30 10:15:43 +01:00
parent ed48a03b21
commit ba3a26ea9f
2 changed files with 59 additions and 6 deletions

View File

@ -1852,7 +1852,7 @@ void QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePriv
only see a press in a new location without any intervening moves. This could
confuse code that is written for a real mouse. The same is true for mouse
release events that change position, see tst_QWidget::touchEventSynthesizedMouseEvent()
auto test.
and tst_QWindow::generatedMouseMove() auto tests.
*/
void QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent *e)
{
@ -1874,7 +1874,7 @@ void QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::Mo
if (!mouseMove && positionChanged) {
QWindowSystemInterfacePrivate::MouseEvent moveEvent(window, e->timestamp,
e->localPos, e->globalPos, e->buttons & ~button, e->modifiers, Qt::NoButton,
e->localPos, e->globalPos, e->buttons ^ button, e->modifiers, Qt::NoButton,
e->nonClientArea ? QEvent::NonClientAreaMouseMove : QEvent::MouseMove,
e->source, e->nonClientArea);
if (e->synthetic())
@ -1977,14 +1977,14 @@ void QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::Mo
const QPointF nativeLocalPoint = QHighDpi::toNativePixels(localPoint, screen);
const QPointF nativeGlobalPoint = QHighDpi::toNativePixels(globalPoint, screen);
QMouseEvent ev(type, nativeLocalPoint, nativeLocalPoint, nativeGlobalPoint,
button, mouse_buttons, e->modifiers, e->source);
button, e->buttons, e->modifiers, e->source);
ev.setTimestamp(e->timestamp);
cursor->pointerEvent(ev);
}
}
#endif
QMouseEvent ev(type, localPoint, localPoint, globalPoint, button, mouse_buttons, e->modifiers, e->source);
QMouseEvent ev(type, localPoint, localPoint, globalPoint, button, e->buttons, e->modifiers, e->source);
ev.setTimestamp(e->timestamp);
if (window->d_func()->blockedByModalWindow && !qApp->d_func()->popupActive()) {
@ -2018,7 +2018,7 @@ void QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::Mo
point.state = Qt::TouchPointPressed;
} else if (type == QEvent::MouseButtonRelease && button == Qt::LeftButton) {
point.state = Qt::TouchPointReleased;
} else if (type == QEvent::MouseMove && (mouse_buttons & Qt::LeftButton)) {
} else if (type == QEvent::MouseMove && (e->buttons & Qt::LeftButton)) {
point.state = Qt::TouchPointMoved;
} else {
return;
@ -2039,7 +2039,7 @@ void QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::Mo
if (!e->window.isNull() || e->nullWindow()) { // QTBUG-36364, check if window closed in response to press
const QEvent::Type doubleClickType = e->nonClientArea ? QEvent::NonClientAreaMouseButtonDblClick : QEvent::MouseButtonDblClick;
QMouseEvent dblClickEvent(doubleClickType, localPoint, localPoint, globalPoint,
button, mouse_buttons, e->modifiers, e->source);
button, e->buttons, e->modifiers, e->source);
dblClickEvent.setTimestamp(e->timestamp);
QGuiApplication::sendSpontaneousEvent(window, &dblClickEvent);
}

View File

@ -106,6 +106,7 @@ private slots:
void flags();
void cleanup();
void testBlockingWindowShownAfterModalDialog();
void generatedMouseMove();
private:
QPoint m_availableTopLeft;
@ -918,6 +919,7 @@ public:
}
}
void mouseMoveEvent(QMouseEvent *event) {
buttonStateInGeneratedMove = event->buttons();
if (ignoreMouse) {
event->ignore();
} else {
@ -999,6 +1001,7 @@ public:
bool ignoreMouse, ignoreTouch;
bool spinLoopWhenPressed;
Qt::MouseButtons buttonStateInGeneratedMove;
};
void tst_QWindow::testInputEvents()
@ -2316,6 +2319,56 @@ void tst_QWindow::testBlockingWindowShownAfterModalDialog()
QVERIFY(normalWindowAfter.gotBlocked);
}
void tst_QWindow::generatedMouseMove()
{
InputTestWindow w;
w.setGeometry(QRect(m_availableTopLeft + QPoint(100, 100), m_testWindowSize));
w.show();
QVERIFY(QTest::qWaitForWindowActive(&w));
QPoint point(10, 10);
QPoint step(2, 2);
QVERIFY(w.mouseMovedCount == 0);
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::NoButton, Qt::NoButton, QEvent::MouseMove);
QCoreApplication::processEvents();
QVERIFY(w.mouseMovedCount == 1);
// Press that does not change position should not generate mouse move
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::LeftButton, Qt::LeftButton, QEvent::MouseButtonPress);
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::LeftButton | Qt::RightButton, Qt::RightButton, QEvent::MouseButtonPress);
QCoreApplication::processEvents();
QVERIFY(w.mouseMovedCount == 1);
// Test moves generated for mouse release
point += step;
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::RightButton, Qt::LeftButton, QEvent::MouseButtonRelease);
QCoreApplication::processEvents();
QVERIFY(w.mouseMovedCount == 2);
QVERIFY(w.buttonStateInGeneratedMove == (Qt::LeftButton | Qt::RightButton));
point += step;
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::NoButton, Qt::RightButton, QEvent::MouseButtonRelease);
QCoreApplication::processEvents();
QVERIFY(w.mouseMovedCount == 3);
QVERIFY(w.buttonStateInGeneratedMove == Qt::RightButton);
// Test moves generated for mouse press
point += step;
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::LeftButton, Qt::LeftButton, QEvent::MouseButtonPress);
QCoreApplication::processEvents();
QVERIFY(w.mouseMovedCount == 4);
QVERIFY(w.buttonStateInGeneratedMove == Qt::NoButton);
point += step;
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::LeftButton | Qt::RightButton, Qt::RightButton, QEvent::MouseButtonPress);
QCoreApplication::processEvents();
QVERIFY(w.mouseMovedCount == 5);
QVERIFY(w.buttonStateInGeneratedMove == Qt::LeftButton);
// Release that does not change position should not generate mouse move
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::LeftButton, Qt::RightButton, QEvent::MouseButtonRelease);
QWindowSystemInterface::handleMouseEvent(&w, point, point, Qt::NoButton, Qt::LeftButton, QEvent::MouseButtonRelease);
QCoreApplication::processEvents();
QVERIFY(w.mouseMovedCount == 5);
}
#include <tst_qwindow.moc>
QTEST_MAIN(tst_QWindow)