macOS: Remove hard-coded logic for determining if key event has text

The normal flow for a keyDown event when sent to a text input enabled
view (NSTextInputClient), is that it's sent through interpretKeyEvents,
which in turn goes through the input methods, and result in either
composing (marking) text, inserting text, or executing a text editing
command such as moving the cursor to the beginning of the line.

  https://apple.co/3qDhwNb

In our case, we prefer to treat "simple" text insertion (non-composed
text) outside of the Qt input method protocol, and send these as normal
key events instead. The same applies when a key event results in a text
editing command that we don't handle.

The problem is that in the latter case, the key event would contain the
text that resulted from e.g. ⌘+K, or one of the function or arrow keys,
which in many cases would not be suitable for inserting into a text
field by a naive client that trusted the text property of the QKeyEvent.

To work around this two exceptions were added; first in 4dbce2a4696081
to ignore text when inside the U+F700-U+F8FF unicode range (arrow keys,
function keys, etc), and second in 933fab137dcaa8 to ignore text for
events that had one or both of the control or command modifiers.

Unfortunately this hard-coded logic was not taking into account that
some keyboard layouts may produce text that match these exceptions,
for example ^⌥+ю with a Russian keyboard layout should result in
inserting a period.

Instead of continuing to add hard-coded exceptions to this logic,
(for example by only filtering out single-modifier events), we
instead use the information that the text input system gives us
via doCommandBySelector to decide whether the key event should
have text or not.

Note: We have similar workarounds for detecting text that is not
suitable for text insertion in other places of Qt, for example in
QInputControl::isAcceptableInput(), but since we can't assume the
client uses QInputControl for their text input needs we need to
filter out the text earlier than that.

Fixes: QTBUG-106393
Task-number: QTBUG-36281
Task-number: QTBUG-35734
Pick-to: 6.4
Change-Id: I7769098cba1c605f6fdb6b23964eb614578724bb
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

3 files changed, 19 insertions, 6 deletions

diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm
index 25cfa97586..b73d186612 100644
--- a/src/plugins/platforms/cocoa/qnsview.mm
+++ b/src/plugins/platforms/cocoa/qnsview.mm
@@ -101,6 +101,7 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QNSViewMouseMoveHelper);
     // Keys
     bool m_lastKeyDead;
     bool m_sendKeyEvent;
+    bool m_sendKeyEventWithoutText;
     NSEvent *m_currentlyInterpretedKeyEvent;
     QSet<quint32> m_acceptedKeyDowns;
 
diff --git a/src/plugins/platforms/cocoa/qnsview_complextext.mm b/src/plugins/platforms/cocoa/qnsview_complextext.mm
index ad5f5a0827..7c50ec7ece 100644
--- a/src/plugins/platforms/cocoa/qnsview_complextext.mm
+++ b/src/plugins/platforms/cocoa/qnsview_complextext.mm
@@ -367,8 +367,18 @@
     // pass the originating key event up the responder chain if applicable.
 
     qCDebug(lcQpaKeys) << "Trying to perform command" << selector;
-    if (![self tryToPerform:selector with:self])
+    if (![self tryToPerform:selector with:self]) {
         m_sendKeyEvent = true;
+
+        // The text input system determined that the key event was not
+        // meant for text insertion, and instead asked us to treat it
+        // as a (possibly noop) command. This typically happens for key
+        // events with either ⌘ or ⌃, function keys such as F1-F35,
+        // arrow keys, etc. We reflect that when sending the key event
+        // later on, by removing the text from the event, so that the
+        // event does not result in text insertion on the client side.
+        m_sendKeyEventWithoutText = true;
+    }
 }
 
 // ------------- Various text properties -------------
diff --git a/src/plugins/platforms/cocoa/qnsview_keys.mm b/src/plugins/platforms/cocoa/qnsview_keys.mm
index 49bbe2308a..cf16cceb83 100644
--- a/src/plugins/platforms/cocoa/qnsview_keys.mm
+++ b/src/plugins/platforms/cocoa/qnsview_keys.mm
@@ -16,6 +16,10 @@
     // We will send a key event unless the input method handles it
     QBoolBlocker sendKeyEventGuard(m_sendKeyEvent, true);
 
+    // Assume we should send key events with text, unless told
+    // otherwise by doCommandBySelector.
+    m_sendKeyEventWithoutText = false;
+
     if (keyEvent.type == QEvent::KeyPress) {
 
         if (m_composingText.isEmpty()) {
@@ -76,6 +80,8 @@
     bool accepted = true;
     if (m_sendKeyEvent && m_composingText.isEmpty()) {
         KeyEvent keyEvent(nsevent);
+        if (m_sendKeyEventWithoutText)
+            keyEvent.text = {};
         qCDebug(lcQpaKeys) << "Sending as" << keyEvent;
         accepted = keyEvent.sendWindowSystemEvent(window);
     }
@@ -229,11 +235,7 @@ KeyEvent::KeyEvent(NSEvent *nsevent)
             key = QAppleKeyMapper::fromCocoaKey(character);
         }
 
-        // Ignore text for the U+F700-U+F8FF range. This is used by Cocoa when
-        // delivering function keys (e.g. arrow keys, backspace, F1-F35, etc.)
-        if (!(modifiers & (Qt::ControlModifier | Qt::MetaModifier))
-            && (character.unicode() < 0xf700 || character.unicode() > 0xf8ff))
-            text = QString::fromNSString(characters);
+        text = QString::fromNSString(characters);
 
         isRepeat = nsevent.ARepeat;
     }