Details
-
User Story
-
Resolution: Unresolved
-
P2: Important
-
None
-
None
-
All
Description
Currently it does this:
Qt::KeyboardModifiers QKeyEvent::modifiers() const { if (key() == Qt::Key_Shift) return Qt::KeyboardModifiers(QInputEvent::modifiers()^Qt::ShiftModifier); if (key() == Qt::Key_Control) return Qt::KeyboardModifiers(QInputEvent::modifiers()^Qt::ControlModifier); if (key() == Qt::Key_Alt) return Qt::KeyboardModifiers(QInputEvent::modifiers()^Qt::AltModifier); if (key() == Qt::Key_Meta) return Qt::KeyboardModifiers(QInputEvent::modifiers()^Qt::MetaModifier); if (key() == Qt::Key_AltGr) return Qt::KeyboardModifiers(QInputEvent::modifiers()^Qt::GroupSwitchModifier); return QInputEvent::modifiers(); }
Which is not only ugly, but also obscure and cause of various bugs.
1) QGuiApplication::keyboardModifiers() does not know about this "^" logic and therefore key press of Control modifier produces:
on press: QFlags<Qt::KeyboardModifier>(NoModifier)
on release: QFlags<Qt::KeyboardModifier>(ControlModifier)
2) macOS [1] and Windows [2] invert the modifiers before sending them to QtGui so they would be reported correctly in QKeyEvent::modifiers(). This is why the modifiers are inverted in (1).
[1] https://codereview.qt-project.org/#/c/56949/
[2] search for "Invert state" in qwindowskeymapper.cpp
X11 also can function without the current logic of QKeyEvent::modifiers():
https://codereview.qt-project.org/#/c/253377/
We can see that none of the major 3 platforms needs this.
3) Code style. The QKeyEvent::modifiers() accessor should simply "return QInputEvent::modifiers()".
4) qtestlib: With this fixed we won't need "the amazing magic" that we currently have in testlib/qtestkeyboard.h::sendKeyEvent():
if (action == Press) { if (modifier & Qt::ShiftModifier) simulateEvent(window, true, Qt::Key_Shift, Qt::KeyboardModifiers(), QString(), false, delay); if (modifier & Qt::ControlModifier) simulateEvent(window, true, Qt::Key_Control, modifier & Qt::ShiftModifier, QString(), false, delay); if (modifier & Qt::AltModifier) simulateEvent(window, true, Qt::Key_Alt, modifier & (Qt::ShiftModifier | Qt::ControlModifier), QString(), false, delay); if (modifier & Qt::MetaModifier) simulateEvent(window, true, Qt::Key_Meta, modifier & (Qt::ShiftModifier | Qt::ControlModifier | Qt::AltModifier), QString(), false, delay);
The XOR logic was added back in 1998:
+/*! + Returns the keyboard modifier flags that existed immediately after + the event occurred. + + \sa state() */ +Qt::ButtonState QKeyEvent::stateAfter() const +{ + if ( key() == Key_Shift ) + return Qt::ButtonState(state()^ShiftButton); + if ( key() == Key_Control ) + return Qt::ButtonState(state()^ControlButton); + if ( key() == Key_Alt ) + return Qt::ButtonState(state()^AltButton); + return state(); +}
It is clearly a legacy that has remained in Qt for way too long.
Attachments
Issue Links
- relates to
-
QTBUG-35968 support native event in qtestlib
-
- Open
-
- mentioned in
-
Page Loading...
Gerrit Reviews
| For Gerrit Dashboard: QTBUG-73826 | ||||||
|---|---|---|---|---|---|---|
| # | Subject | Branch | Project | Status | CR | V |
| 253377,1 | xcb: WIP: prepare for QKeyEvent::modifiers() changes | dev | qt/qtbase | Status: DEFERRED | -2 | 0 |
| 253469,2 | remove obscure comment | 5.13 | qt/qtbase | Status: MERGED | +2 | 0 |