macOS: Filter out and prioritise key combinations that produce the same key

An incoming key event with a set of modifiers can potentially match
a range of key combinations, depending on how the event's modifiers
are combined to produce "intermediate" representations of the event.

For example, given a normal US keyboard layout, the virtual key
23 combined with the Alt (⌥) and Shift (⇧) modifiers, can map
to the following key combinations:

  - Alt+Shift+5 (Fully expressed combination)
  - Alt+% (Shift consumed to produce %)
  - Shift+∞ (Alt consumed to produce ∞)
  - fi (Shift and Alt consumed to produce fi)

But in other cases the intermediate modifier combinations
produce the same key/symbol as other modifier combinations.

For example, pressing  Alt (⌥) and Shift (⇧) with the 'c'
key on a US layout will produce:

  - Alt+Shift+C (Fully expressed combination)
  - Shift+Ç (Alt consumed to produce Ç)
  - Ç (Shift and Alt consumed to produce Ç)

In this case, we don't want to reflect the standalone 'Ç',
as that has already been reflected in the more direct form
via Shift+Ç. Consuming the additional Shift modifier does
not produce any additional symbols.

The same can happen without the number of modifiers being
different, in case two modifiers produce the same symbol.
In this case we want to prioritize Command over Option
over Control over Shift.

There is similar logic in the Windows and XKB key mappers,
and the implementation in the Apple key mapper has been
adapted from the Windows key mapper.

Task-number: QTBUG-67200
Task-number: QTBUG-38137
Change-Id: I4f1aeebac78a5393f8da804b53cf588f7c802c1b
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>

1 files changed, 32 insertions, 3 deletions

diff --git a/src/gui/platform/darwin/qapplekeymapper.mm b/src/gui/platform/darwin/qapplekeymapper.mm
index 7863e53ce2..2601c13868 100644
--- a/src/gui/platform/darwin/qapplekeymapper.mm
+++ b/src/gui/platform/darwin/qapplekeymapper.mm
@@ -530,8 +530,6 @@ QList<QKeyCombination> QAppleKeyMapper::possibleKeyCombinations(const QKeyEvent
     // FIXME: We only compute the first 8 combinations. Why?
     for (int i = 1; i < 8; ++i) {
         auto keyAfterApplyingModifiers = keyMap[i];
-        if (keyAfterApplyingModifiers == unmodifiedKey)
-            continue;
         if (!keyAfterApplyingModifiers)
              continue;
 
@@ -542,8 +540,39 @@ QList<QKeyCombination> QAppleKeyMapper::possibleKeyCombinations(const QKeyEvent
             // If the event includes more modifiers than the candidate they
             // will need to be included in the resulting key combination.
             auto additionalModifiers = eventModifiers & ~candidateModifiers;
-            ret << QKeyCombination::fromCombined(
+
+            auto keyCombination = QKeyCombination::fromCombined(
                 int(additionalModifiers) + int(keyAfterApplyingModifiers));
+
+            // If there's an existing key combination with the same key,
+            // but a different set of modifiers, we want to choose only
+            // one of them, by priority (see below).
+            const auto existingCombination = std::find_if(
+                ret.begin(), ret.end(), [&](auto existingCombination) {
+                    return existingCombination.key() == keyAfterApplyingModifiers;
+                });
+
+            if (existingCombination != ret.end()) {
+                // We prioritize the combination with the more specific
+                // modifiers. In the case where the number of modifiers
+                // are the same, we want to prioritize Command over Option
+                // over Control over Shift. Unfortunately the order (and
+                // hence value) of the modifiers in Qt::KeyboardModifier
+                // does not match our preferred order when Control and
+                // Meta is switched, but we can work around that by
+                // explicitly swapping the modifiers and using that
+                // for the comparison. This also works when the
+                // Qt::AA_MacDontSwapCtrlAndMeta application attribute
+                // is set, as the incoming modifiers are then left
+                // as is, and we can still trust the order.
+                auto existingModifiers = swapModifiersIfNeeded(existingCombination->keyboardModifiers());
+                auto replacementModifiers = swapModifiersIfNeeded(additionalModifiers);
+                if (replacementModifiers > existingModifiers)
+                    *existingCombination = keyCombination;
+            } else {
+                // All is good, no existing combination has this key
+                ret << keyCombination;
+            }
         }
     }