Unverified Commit 54095e1b authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Make modifier key side handling consistent among platforms (#63417)

This makes the processing of RawKeyboardEventData.getModifierSide consistent among the platforms.

Before this change, some platforms (Android) didn't handle the case where the "either" flag was set, but no side flag was set (e.g. "shift" was set, but not "shift left"), and instead said that no keys were down by returning null (which was wrong).

Some platforms (Linux, Windows) also returned KeyboardSide.any from getModifierSide, when the contract says that it will never return KeyboardSide.any. Those methods now return KeyboardSide.all in the case where no side is specified, as macOS and Fuchsia already did.

Now, all platforms will return KeyboardSide.all from getModifierSide when it's not clear which side the key was on.
parent 49fac9a8
......@@ -165,11 +165,26 @@ abstract class RawKeyEventData {
/// Returns a map of modifier keys that were pressed at the time of this
/// event, and the keyboard side or sides that the key was on.
Map<ModifierKey, KeyboardSide?> get modifiersPressed {
final Map<ModifierKey, KeyboardSide?> result = <ModifierKey, KeyboardSide?>{};
Map<ModifierKey, KeyboardSide> get modifiersPressed {
final Map<ModifierKey, KeyboardSide> result = <ModifierKey, KeyboardSide>{};
for (final ModifierKey key in ModifierKey.values) {
if (isModifierPressed(key)) {
result[key] = getModifierSide(key);
final KeyboardSide? side = getModifierSide(key);
if (side != null) {
result[key] = side;
}
assert((){
if (side == null) {
debugPrint('Raw key data is returning inconsistent information for '
'pressed modifiers. isModifierPressed returns true for $key '
'being pressed, but when getModifierSide is called, it says '
'that no modifiers are pressed.');
if (this is RawKeyEventDataAndroid) {
debugPrint('Android raw key metaState: ${(this as RawKeyEventDataAndroid).metaState}');
}
}
return true;
}());
}
}
return result;
......@@ -660,10 +675,21 @@ class RawKeyboard {
final Map<ModifierKey, KeyboardSide?> modifiersPressed = event.data.modifiersPressed;
final Map<PhysicalKeyboardKey, LogicalKeyboardKey> modifierKeys = <PhysicalKeyboardKey, LogicalKeyboardKey>{};
for (final ModifierKey key in modifiersPressed.keys) {
final Set<PhysicalKeyboardKey> mappedKeys = _modifierKeyMap[_ModifierSidePair(key, modifiersPressed[key])]!;
assert(mappedKeys != null,
'Platform key support for ${Platform.operatingSystem} is '
'producing unsupported modifier combinations.');
final Set<PhysicalKeyboardKey>? mappedKeys = _modifierKeyMap[_ModifierSidePair(key, modifiersPressed[key])];
assert((){
if (mappedKeys == null) {
debugPrint('Platform key support for ${Platform.operatingSystem} is '
'producing unsupported modifier combinations for '
'modifier $key on side ${modifiersPressed[key]}.');
if (event.data is RawKeyEventDataAndroid) {
debugPrint('Android raw key metaState: ${(event.data as RawKeyEventDataAndroid).metaState}');
}
}
return true;
}());
if (mappedKeys == null) {
continue;
}
for (final PhysicalKeyboardKey physicalModifier in mappedKeys) {
modifierKeys[physicalModifier] = _allModifiers[physicalModifier]!;
}
......
......@@ -260,7 +260,7 @@ class RawKeyEventDataAndroid extends RawKeyEventData {
@override
KeyboardSide? getModifierSide(ModifierKey key) {
KeyboardSide? findSide(int leftMask, int rightMask) {
KeyboardSide? findSide(int anyMask, int leftMask, int rightMask) {
final int combinedMask = leftMask | rightMask;
final int combined = metaState & combinedMask;
if (combined == leftMask) {
......@@ -270,18 +270,24 @@ class RawKeyEventDataAndroid extends RawKeyEventData {
} else if (combined == combinedMask) {
return KeyboardSide.all;
}
// If the platform code sets the "any" modifier, but not a specific side,
// then we return "all", assuming that there is only one of that modifier
// key on the keyboard.
if (metaState & anyMask != 0) {
return KeyboardSide.all;
}
return null;
}
switch (key) {
case ModifierKey.controlModifier:
return findSide(modifierLeftControl, modifierRightControl);
return findSide(modifierControl, modifierLeftControl, modifierRightControl);
case ModifierKey.shiftModifier:
return findSide(modifierLeftShift, modifierRightShift);
return findSide(modifierShift, modifierLeftShift, modifierRightShift);
case ModifierKey.altModifier:
return findSide(modifierLeftAlt, modifierRightAlt);
return findSide(modifierAlt, modifierLeftAlt, modifierRightAlt);
case ModifierKey.metaModifier:
return findSide(modifierLeftMeta, modifierRightMeta);
return findSide(modifierMeta, modifierLeftMeta, modifierRightMeta);
case ModifierKey.capsLockModifier:
case ModifierKey.numLockModifier:
case ModifierKey.scrollLockModifier:
......
......@@ -135,13 +135,13 @@ class RawKeyEventDataFuchsia extends RawKeyEventData {
@override
KeyboardSide? getModifierSide(ModifierKey key) {
KeyboardSide? findSide(int leftMask, int rightMask, int combinedMask) {
final int combined = modifiers & combinedMask;
KeyboardSide? findSide(int anyMask, int leftMask, int rightMask) {
final int combined = modifiers & anyMask;
if (combined == leftMask) {
return KeyboardSide.left;
} else if (combined == rightMask) {
return KeyboardSide.right;
} else if (combined == combinedMask) {
} else if (combined == anyMask) {
return KeyboardSide.all;
}
return null;
......@@ -149,13 +149,13 @@ class RawKeyEventDataFuchsia extends RawKeyEventData {
switch (key) {
case ModifierKey.controlModifier:
return findSide(modifierLeftControl, modifierRightControl, modifierControl);
return findSide(modifierControl, modifierLeftControl, modifierRightControl, );
case ModifierKey.shiftModifier:
return findSide(modifierLeftShift, modifierRightShift, modifierShift);
return findSide(modifierShift, modifierLeftShift, modifierRightShift);
case ModifierKey.altModifier:
return findSide(modifierLeftAlt, modifierRightAlt, modifierAlt);
return findSide(modifierAlt, modifierLeftAlt, modifierRightAlt);
case ModifierKey.metaModifier:
return findSide(modifierLeftMeta, modifierRightMeta, modifierMeta);
return findSide(modifierMeta, modifierLeftMeta, modifierRightMeta);
case ModifierKey.capsLockModifier:
return (modifiers & modifierCapsLock == 0) ? null : KeyboardSide.all;
case ModifierKey.numLockModifier:
......
......@@ -286,22 +286,11 @@ class GLFWKeyHelper with KeyHelper {
@override
KeyboardSide getModifierSide(ModifierKey key) {
switch (key) {
case ModifierKey.controlModifier:
case ModifierKey.shiftModifier:
case ModifierKey.altModifier:
case ModifierKey.metaModifier:
// Neither GLFW or X11 provide a distinction between left and right modifiers, so defaults to KeyboardSide.any.
// Neither GLFW nor X11 provide a distinction between left and right
// modifiers, so defaults to KeyboardSide.all.
// https://code.woboq.org/qt5/include/X11/X.h.html#_M/ShiftMask
return KeyboardSide.any;
case ModifierKey.capsLockModifier:
case ModifierKey.numLockModifier:
case ModifierKey.functionModifier:
case ModifierKey.symbolModifier:
case ModifierKey.scrollLockModifier:
return KeyboardSide.all;
}
}
@override
LogicalKeyboardKey? numpadKey(int keyCode) {
......@@ -435,22 +424,11 @@ class GtkKeyHelper with KeyHelper {
@override
KeyboardSide getModifierSide(ModifierKey key) {
switch (key) {
case ModifierKey.controlModifier:
case ModifierKey.shiftModifier:
case ModifierKey.altModifier:
case ModifierKey.metaModifier:
// Neither GTK or X11 provide a distinction between left and right modifiers, so defaults to KeyboardSide.any.
// Neither GTK nor X11 provide a distinction between left and right
// modifiers, so defaults to KeyboardSide.all.
// https://code.woboq.org/qt5/include/X11/X.h.html#_M/ShiftMask
return KeyboardSide.any;
case ModifierKey.capsLockModifier:
case ModifierKey.numLockModifier:
case ModifierKey.functionModifier:
case ModifierKey.symbolModifier:
case ModifierKey.scrollLockModifier:
return KeyboardSide.all;
}
}
@override
LogicalKeyboardKey? numpadKey(int keyCode) {
......
......@@ -184,7 +184,7 @@ class RawKeyEventDataMacOs extends RawKeyEventData {
@override
KeyboardSide? getModifierSide(ModifierKey key) {
KeyboardSide? findSide(int leftMask, int rightMask, int anyMask) {
KeyboardSide? findSide(int anyMask, int leftMask, int rightMask) {
final int combinedMask = leftMask | rightMask;
final int combined = modifiers & combinedMask;
if (combined == leftMask) {
......@@ -194,7 +194,8 @@ class RawKeyEventDataMacOs extends RawKeyEventData {
} else if (combined == combinedMask || modifiers & (combinedMask | anyMask) == anyMask) {
// Handles the case where macOS supplies just the "either" modifier
// flag, but not the left/right flag. (e.g. modifierShift but not
// modifierLeftShift).
// modifierLeftShift), or if left and right flags are provided, but not
// the "either" modifier flag.
return KeyboardSide.all;
}
return null;
......@@ -202,13 +203,13 @@ class RawKeyEventDataMacOs extends RawKeyEventData {
switch (key) {
case ModifierKey.controlModifier:
return findSide(modifierLeftControl, modifierRightControl, modifierControl);
return findSide(modifierControl, modifierLeftControl, modifierRightControl);
case ModifierKey.shiftModifier:
return findSide(modifierLeftShift, modifierRightShift, modifierShift);
return findSide(modifierShift, modifierLeftShift, modifierRightShift);
case ModifierKey.altModifier:
return findSide(modifierLeftOption, modifierRightOption, modifierOption);
return findSide(modifierOption, modifierLeftOption, modifierRightOption);
case ModifierKey.metaModifier:
return findSide(modifierLeftCommand, modifierRightCommand, modifierCommand);
return findSide(modifierCommand, modifierLeftCommand, modifierRightCommand);
case ModifierKey.capsLockModifier:
case ModifierKey.numLockModifier:
case ModifierKey.scrollLockModifier:
......
......@@ -168,13 +168,11 @@ class RawKeyEventDataWindows extends RawKeyEventData {
return KeyboardSide.left;
} else if (combined == rightMask) {
return KeyboardSide.right;
} else if (combined == combinedMask) {
return KeyboardSide.all;
} else if (modifiers & (combinedMask | anyMask) == anyMask) {
} else if (combined == combinedMask || modifiers & (combinedMask | anyMask) == anyMask) {
// Handles the case where Windows supplies just the "either" modifier
// flag, but not the left/right flag. (e.g. modifierShift but not
// modifierLeftShift).
return KeyboardSide.any;
return KeyboardSide.all;
}
return null;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment