From 9f9b9d6057efed3fad2a930094d0538d40cdce07 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 Nov 2021 14:15:51 -0800 Subject: [PATCH 1/6] Attempt --- .../Source/FlutterEmbedderKeyResponder.mm | 68 ++++++++++--------- .../FlutterEmbedderKeyResponderUnittests.mm | 43 ++++++++++++ 2 files changed, 78 insertions(+), 33 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index dd65f2e97285c..304e428e6d553 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -29,23 +29,15 @@ static NSUInteger lowestSetBit(NSUInteger bitmask) { /** * Whether a string represents a control character. */ -static bool IsControlCharacter(NSUInteger length, NSString* label) { - if (length > 1) { - return false; - } - unichar codeUnit = [label characterAtIndex:0]; - return (codeUnit <= 0x1f && codeUnit >= 0x00) || (codeUnit >= 0x7f && codeUnit <= 0x9f); +static bool IsControlCharacter(uint64_t character) { + return (character <= 0x1f && character >= 0x00) || (character >= 0x7f && character <= 0x9f); } /** * Whether a string represents an unprintable key. */ -static bool IsUnprintableKey(NSUInteger length, NSString* label) { - if (length > 1) { - return false; - } - unichar codeUnit = [label characterAtIndex:0]; - return codeUnit >= 0xF700 && codeUnit <= 0xF8FF; +static bool IsUnprintableKey(uint64_t character) { + return character >= 0xF700 && character <= 0xF8FF; } /** @@ -125,30 +117,38 @@ static uint64_t GetLogicalKeyForEvent(NSEvent* event, uint64_t physicalKey) { return fromKeyCode.unsignedLongLongValue; } - NSString* keyLabel = event.charactersIgnoringModifiers; - NSUInteger keyLabelLength = [keyLabel length]; - // If this key is printable, generate the logical key from its Unicode - // value. Control keys such as ESC, CTRL, and SHIFT are not printable. HOME, - // DEL, arrow keys, and function keys are considered modifier function keys, - // which generate invalid Unicode scalar values. - if (keyLabelLength != 0 && !IsControlCharacter(keyLabelLength, keyLabel) && - !IsUnprintableKey(keyLabelLength, keyLabel)) { - // Given that charactersIgnoringModifiers can contain a string of arbitrary - // length, limit to a maximum of two Unicode scalar values. It is unlikely - // that a keyboard would produce a code point bigger than 32 bits, but it is - // still worth defending against this case. - NSCAssert((keyLabelLength < 2), @"Unexpected long key label: |%@|.", keyLabel); - - uint64_t codeUnit = (uint64_t)[keyLabel characterAtIndex:0]; - if (keyLabelLength == 2) { - uint64_t secondCode = (uint64_t)[keyLabel characterAtIndex:1]; - codeUnit = (codeUnit << 16) | secondCode; + // Convert `charactersIgnoringModifiers` to UTF32. + NSString* keyLabelUtf16 = event.charactersIgnoringModifiers; + + // Check if this key is a single printable character, which will be used to + // generate the logical key from its Unicode value. This excludes: + // + // - Control keys such as ESC, CTRL, and SHIFT, which are not printable. + // - HOME, DEL, arrow keys, and function keys, which are considered modifier function + // keys and generate invalid Unicode scalar values. + // - A string of multiple characters, which has been observed in + // https://github.com/flutter/flutter/issues/82673. + // + // TODO(dkwingsmt): Theoretically control keys and modifier keys should have + // been converted in keyCodeToLogicalKey. We should convert it into an assert + // instead. I didn't do so for now because we're still stablizing the logic. + uint32_t character = 0; + if (keyLabelUtf16.length != 0 && [keyLabelUtf16 canBeConvertedToEncoding:NSUTF32StringEncoding]) { + NSData* keyLabelData = [keyLabelUtf16 dataUsingEncoding:NSUTF32StringEncoding]; + if (keyLabelData.length == 4) { // 4 bytes constitutes 1 UTF32 character + uint32_t keyLabel = *reinterpret_cast(keyLabelData.bytes); + if (!IsControlCharacter(keyLabel) && !IsUnprintableKey(keyLabel)) { + NSCAssert(keyLabel <= 0x10FFFF, @"Out of range keylabel 0x%x", keyLabel); + character = keyLabel; + } } - return KeyOfPlane(toLower(codeUnit), kUnicodePlane); + } + if (character != 0) { + return KeyOfPlane(toLower(character), kUnicodePlane); } - // This is a non-printable key that is unrecognized, so a new code is minted - // to the macOS plane. + // We can't represent this key with a single printable unicode, so a new code + // is minted to the macOS plane. return KeyOfPlane(event.keyCode, kMacosPlane); } @@ -489,6 +489,8 @@ - (void)handleEvent:(NSEvent*)event callback:(FlutterAsyncKeyCallback)callback { NSAssert(callback != nil, @"The callback must not be nil."); FlutterKeyCallbackGuard* guardedCallback = [[FlutterKeyCallbackGuard alloc] initWithCallback:callback]; + NSLog(@"Type [%lu] keycode 0x%x char |%@| charIM |%@|", event.type, + event.keyCode, event.characters, event.charactersIgnoringModifiers); switch (event.type) { case NSEventTypeKeyDown: [self handleDownEvent:event callback:guardedCallback]; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm index 7e1012569abb6..cd1d41e63543a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm @@ -265,6 +265,49 @@ - (void)dealloc { [events removeAllObjects]; } +TEST(FlutterEmbedderKeyResponderUnittests, MultipleCharacters) { + __block NSMutableArray* events = [[NSMutableArray alloc] init]; + FlutterKeyEvent* event; + + FlutterEmbedderKeyResponder* responder = [[FlutterEmbedderKeyResponder alloc] + initWithSendEvent:^(const FlutterKeyEvent& event, _Nullable FlutterKeyEventCallback callback, + _Nullable _VoidPtr user_data) { + [events addObject:[[TestKeyEvent alloc] initWithEvent:&event + callback:callback + userData:user_data]]; + }]; + + [responder + handleEvent:keyEvent(NSEventTypeKeyDown, 100, @"àn", @"àn", FALSE, kKeyCodeKeyA) + callback:^(BOOL handled){ + }]; + + EXPECT_EQ([events count], 1u); + event = [events lastObject].data; + EXPECT_EQ(event->type, kFlutterKeyEventTypeDown); + EXPECT_EQ(event->physical, kPhysicalKeyA); + EXPECT_EQ(event->logical, kLogicalAltRight); + EXPECT_STREQ(event->character, "àn"); + EXPECT_EQ(event->synthesized, false); + + [events removeAllObjects]; + + [responder + handleEvent:keyEvent(NSEventTypeKeyUp, 100, @"a", @"a", FALSE, kKeyCodeKeyA) + callback:^(BOOL handled){ + }]; + + EXPECT_EQ([events count], 1u); + event = [events lastObject].data; + EXPECT_EQ(event->type, kFlutterKeyEventTypeUp); + EXPECT_EQ(event->physical, kPhysicalKeyA); + EXPECT_EQ(event->logical, kLogicalKeyW); + EXPECT_STREQ(event->character, nullptr); + EXPECT_EQ(event->synthesized, false); + + [events removeAllObjects]; +} + TEST(FlutterEmbedderKeyResponderUnittests, IgnoreDuplicateDownEvent) { __block NSMutableArray* events = [[NSMutableArray alloc] init]; __block BOOL last_handled = TRUE; From 39af072f98f2be9bcad4cfef328ffb00900f1707 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 Nov 2021 15:26:49 -0800 Subject: [PATCH 2/6] Write my own decode --- .../Source/FlutterEmbedderKeyResponder.mm | 58 ++++++++++++++++--- .../FlutterEmbedderKeyResponderUnittests.mm | 8 +-- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index 304e428e6d553..37aea2d22fec5 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -105,6 +105,36 @@ static uint64_t toLower(uint64_t n) { return n; } +// Decode a UTF-16 sequence to an array of char32 (UTF-32). +// +// The returned character array must be cleared using delete[]. +// +// See https://en.wikipedia.org/wiki/UTF-16#Description. +static uint32_t* DecodeUtf16(NSString* target, size_t* out_length) { + // The result always has a length less or equal to target. + size_t result_pos = 0; + uint32_t* result = new uint32_t[target.length]; + uint16_t high_surrogate = 0; + for (NSUInteger target_pos = 0; target_pos < target.length; target_pos += 1) { + uint16_t codeUnit = [target characterAtIndex:target_pos]; + // BMP + if (codeUnit <= 0xD7FF || codeUnit >= 0xE000) { + result[result_pos] = codeUnit; + result_pos += 1; + // High surrogates + } else if (codeUnit <= 0xDBFF) { + high_surrogate = codeUnit - 0xD800; + // Low surrogates + } else { + uint16_t low_surrogate = codeUnit - 0xDC00; + result[result_pos] = (high_surrogate << 10) + low_surrogate + 0x10000; + result_pos += 1; + } + } + *out_length = result_pos; + return result; +} + /** * Returns the logical key of a KeyUp or KeyDown event. * @@ -133,13 +163,23 @@ static uint64_t GetLogicalKeyForEvent(NSEvent* event, uint64_t physicalKey) { // been converted in keyCodeToLogicalKey. We should convert it into an assert // instead. I didn't do so for now because we're still stablizing the logic. uint32_t character = 0; - if (keyLabelUtf16.length != 0 && [keyLabelUtf16 canBeConvertedToEncoding:NSUTF32StringEncoding]) { - NSData* keyLabelData = [keyLabelUtf16 dataUsingEncoding:NSUTF32StringEncoding]; - if (keyLabelData.length == 4) { // 4 bytes constitutes 1 UTF32 character - uint32_t keyLabel = *reinterpret_cast(keyLabelData.bytes); - if (!IsControlCharacter(keyLabel) && !IsUnprintableKey(keyLabel)) { - NSCAssert(keyLabel <= 0x10FFFF, @"Out of range keylabel 0x%x", keyLabel); - character = keyLabel; + if (keyLabelUtf16.length != 0) { + for (NSUInteger i = 0; i < keyLabelUtf16.length; ++i) { + printf("0x%04x ", [keyLabelUtf16 characterAtIndex:i]); + } + NSLog(@""); + size_t keyLabelLength; + uint32_t* keyLabel = DecodeUtf16(keyLabelUtf16, &keyLabelLength); + if (keyLabelLength == 1) { + for (NSUInteger i = 0; i < keyLabelLength; ++i) { + printf("0x%04x ", keyLabel[i]); + } + NSLog(@""); + uint32_t keyLabelChar = *keyLabel; + delete[] keyLabel; + if (!IsControlCharacter(keyLabelChar) && !IsUnprintableKey(keyLabelChar)) { + NSCAssert(keyLabelChar <= 0x10FFFF, @"Out of range keylabel 0x%x", keyLabelChar); + character = keyLabelChar; } } } @@ -489,8 +529,8 @@ - (void)handleEvent:(NSEvent*)event callback:(FlutterAsyncKeyCallback)callback { NSAssert(callback != nil, @"The callback must not be nil."); FlutterKeyCallbackGuard* guardedCallback = [[FlutterKeyCallbackGuard alloc] initWithCallback:callback]; - NSLog(@"Type [%lu] keycode 0x%x char |%@| charIM |%@|", event.type, - event.keyCode, event.characters, event.charactersIgnoringModifiers); + // NSLog(@"Type [%lu] keycode 0x%x char |%@| charIM |%@|", event.type, + // event.keyCode, event.characters, event.charactersIgnoringModifiers); switch (event.type) { case NSEventTypeKeyDown: [self handleDownEvent:event callback:guardedCallback]; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm index cd1d41e63543a..a4a6474455466 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm @@ -278,7 +278,7 @@ - (void)dealloc { }]; [responder - handleEvent:keyEvent(NSEventTypeKeyDown, 100, @"àn", @"àn", FALSE, kKeyCodeKeyA) + handleEvent:keyEvent(NSEventTypeKeyDown, 0, @"àn", @"àn", FALSE, kKeyCodeKeyA) callback:^(BOOL handled){ }]; @@ -286,14 +286,14 @@ - (void)dealloc { event = [events lastObject].data; EXPECT_EQ(event->type, kFlutterKeyEventTypeDown); EXPECT_EQ(event->physical, kPhysicalKeyA); - EXPECT_EQ(event->logical, kLogicalAltRight); + EXPECT_EQ(event->logical, 0x1400000000ull); EXPECT_STREQ(event->character, "àn"); EXPECT_EQ(event->synthesized, false); [events removeAllObjects]; [responder - handleEvent:keyEvent(NSEventTypeKeyUp, 100, @"a", @"a", FALSE, kKeyCodeKeyA) + handleEvent:keyEvent(NSEventTypeKeyUp, 0, @"a", @"a", FALSE, kKeyCodeKeyA) callback:^(BOOL handled){ }]; @@ -301,7 +301,7 @@ - (void)dealloc { event = [events lastObject].data; EXPECT_EQ(event->type, kFlutterKeyEventTypeUp); EXPECT_EQ(event->physical, kPhysicalKeyA); - EXPECT_EQ(event->logical, kLogicalKeyW); + EXPECT_EQ(event->logical, 0x1400000000ull); EXPECT_STREQ(event->character, nullptr); EXPECT_EQ(event->synthesized, false); From 0c0ea6efdaa18011a976389e29366d68347702d6 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 Nov 2021 15:50:32 -0800 Subject: [PATCH 3/6] Doc --- .../macos/framework/Source/FlutterEmbedderKeyResponder.mm | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index 37aea2d22fec5..6721ec2c8073c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -107,9 +107,13 @@ static uint64_t toLower(uint64_t n) { // Decode a UTF-16 sequence to an array of char32 (UTF-32). // -// The returned character array must be cleared using delete[]. +// See https://en.wikipedia.org/wiki/UTF-16#Description for the algorithm. // -// See https://en.wikipedia.org/wiki/UTF-16#Description. +// The returned character array must be cleared with delete[]. The length of +// the result is stored in `out_length`. +// +// Although NSString has a dataUsingEncoding method, we implement our own +// because dataUsingEncoding outputs redundant characters for unknown reasons. static uint32_t* DecodeUtf16(NSString* target, size_t* out_length) { // The result always has a length less or equal to target. size_t result_pos = 0; From 4f7c873ad63ce5978fe0d84f62c2e9d27ac92c71 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 24 Nov 2021 14:01:12 -0800 Subject: [PATCH 4/6] Format --- .../Source/FlutterEmbedderKeyResponder.mm | 4 ++-- .../Source/FlutterEmbedderKeyResponderUnittests.mm | 14 ++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index 6721ec2c8073c..6aaa6291b6306 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -125,10 +125,10 @@ static uint64_t toLower(uint64_t n) { if (codeUnit <= 0xD7FF || codeUnit >= 0xE000) { result[result_pos] = codeUnit; result_pos += 1; - // High surrogates + // High surrogates } else if (codeUnit <= 0xDBFF) { high_surrogate = codeUnit - 0xD800; - // Low surrogates + // Low surrogates } else { uint16_t low_surrogate = codeUnit - 0xDC00; result[result_pos] = (high_surrogate << 10) + low_surrogate + 0x10000; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm index a4a6474455466..f61f1c99308b2 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm @@ -277,10 +277,9 @@ - (void)dealloc { userData:user_data]]; }]; - [responder - handleEvent:keyEvent(NSEventTypeKeyDown, 0, @"àn", @"àn", FALSE, kKeyCodeKeyA) - callback:^(BOOL handled){ - }]; + [responder handleEvent:keyEvent(NSEventTypeKeyDown, 0, @"àn", @"àn", FALSE, kKeyCodeKeyA) + callback:^(BOOL handled){ + }]; EXPECT_EQ([events count], 1u); event = [events lastObject].data; @@ -292,10 +291,9 @@ - (void)dealloc { [events removeAllObjects]; - [responder - handleEvent:keyEvent(NSEventTypeKeyUp, 0, @"a", @"a", FALSE, kKeyCodeKeyA) - callback:^(BOOL handled){ - }]; + [responder handleEvent:keyEvent(NSEventTypeKeyUp, 0, @"a", @"a", FALSE, kKeyCodeKeyA) + callback:^(BOOL handled){ + }]; EXPECT_EQ([events count], 1u); event = [events lastObject].data; From 20326402680d0842d71517fc6bb3ab7c0bd95faf Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 30 Nov 2021 14:41:53 -0800 Subject: [PATCH 5/6] Fix comment and log --- .../Source/FlutterEmbedderKeyResponder.mm | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index 6aaa6291b6306..eac21033eee87 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -154,37 +154,23 @@ static uint64_t GetLogicalKeyForEvent(NSEvent* event, uint64_t physicalKey) { // Convert `charactersIgnoringModifiers` to UTF32. NSString* keyLabelUtf16 = event.charactersIgnoringModifiers; - // Check if this key is a single printable character, which will be used to - // generate the logical key from its Unicode value. This excludes: + // Check if this key is a single character, which will be used to generate the + // logical key from its Unicode value. // - // - Control keys such as ESC, CTRL, and SHIFT, which are not printable. - // - HOME, DEL, arrow keys, and function keys, which are considered modifier function - // keys and generate invalid Unicode scalar values. - // - A string of multiple characters, which has been observed in - // https://github.com/flutter/flutter/issues/82673. - // - // TODO(dkwingsmt): Theoretically control keys and modifier keys should have - // been converted in keyCodeToLogicalKey. We should convert it into an assert - // instead. I didn't do so for now because we're still stablizing the logic. + // Multi-char keys will be minted onto the macOS plane because there are no + // meaningful values for them. Control keys and unprintable keys have been + // converted by `keyCodeToLogicalKey` earlier. uint32_t character = 0; if (keyLabelUtf16.length != 0) { - for (NSUInteger i = 0; i < keyLabelUtf16.length; ++i) { - printf("0x%04x ", [keyLabelUtf16 characterAtIndex:i]); - } - NSLog(@""); size_t keyLabelLength; uint32_t* keyLabel = DecodeUtf16(keyLabelUtf16, &keyLabelLength); if (keyLabelLength == 1) { - for (NSUInteger i = 0; i < keyLabelLength; ++i) { - printf("0x%04x ", keyLabel[i]); - } - NSLog(@""); uint32_t keyLabelChar = *keyLabel; delete[] keyLabel; - if (!IsControlCharacter(keyLabelChar) && !IsUnprintableKey(keyLabelChar)) { - NSCAssert(keyLabelChar <= 0x10FFFF, @"Out of range keylabel 0x%x", keyLabelChar); - character = keyLabelChar; - } + NSCAssert(!IsControlCharacter(keyLabelChar) && !IsUnprintableKey(keyLabelChar), + @"Unexpected control or unprintable keylabel 0x%x", keyLabelChar); + NSCAssert(keyLabelChar <= 0x10FFFF, @"Out of range keylabel 0x%x", keyLabelChar); + character = keyLabelChar; } } if (character != 0) { @@ -533,8 +519,6 @@ - (void)handleEvent:(NSEvent*)event callback:(FlutterAsyncKeyCallback)callback { NSAssert(callback != nil, @"The callback must not be nil."); FlutterKeyCallbackGuard* guardedCallback = [[FlutterKeyCallbackGuard alloc] initWithCallback:callback]; - // NSLog(@"Type [%lu] keycode 0x%x char |%@| charIM |%@|", event.type, - // event.keyCode, event.characters, event.charactersIgnoringModifiers); switch (event.type) { case NSEventTypeKeyDown: [self handleDownEvent:event callback:guardedCallback]; From 62afdc152004abe8c14af72fc4b85009a1ea6c8f Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 30 Nov 2021 16:35:25 -0800 Subject: [PATCH 6/6] Update FlutterEmbedderKeyResponder.mm --- .../macos/framework/Source/FlutterEmbedderKeyResponder.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index eac21033eee87..1a0747b3b59be 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -109,7 +109,7 @@ static uint64_t toLower(uint64_t n) { // // See https://en.wikipedia.org/wiki/UTF-16#Description for the algorithm. // -// The returned character array must be cleared with delete[]. The length of +// The returned character array must be deallocated with delete[]. The length of // the result is stored in `out_length`. // // Although NSString has a dataUsingEncoding method, we implement our own