[macOS] Synchronise modifiers from mouse events for RawKeyboard (#46230)

Fixes flutter/flutter#135349 This has been done for FlutterEmbedderKeyResponder in #37870, but has not been implemented for FlutterChannelKeyResponder, which results in RawKeyboard being out of sync with HardwareKeyboard. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
flutter · Sep 27, 2023 · 3ed6e7f · 3ed6e7f
1 parent 23b0f5b
commit 3ed6e7f
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 8 deletions.
diff --git a/shell/platform/darwin/macos/framework/Source/FlutterChannelKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterChannelKeyResponder.mm
@@ -40,6 +40,74 @@ - (nonnull instancetype)initWithChannel:(nonnull FlutterBasicMessageChannel*)cha
   return self;
 }
 
+/// Checks single modifier flag from event flags and sends appropriate key event
+/// if it is different from the previous state.
+- (void)checkModifierFlag:(NSUInteger)targetMask
+            forEventFlags:(NSEventModifierFlags)eventFlags
+                  keyCode:(NSUInteger)keyCode
+                timestamp:(NSTimeInterval)timestamp {
+  NSAssert((targetMask & (targetMask - 1)) == 0, @"targetMask must only have one bit set");
+  if ((eventFlags & targetMask) != (_previouslyPressedFlags & targetMask)) {
+    uint64_t newFlags = (_previouslyPressedFlags & ~targetMask) | (eventFlags & targetMask);
+
+    // Sets combined flag if either left or right modifier is pressed, unsets otherwise.
+    auto updateCombinedFlag = [&](uint64_t side1, uint64_t side2, NSEventModifierFlags flag) {
+      if (newFlags & (side1 | side2)) {
+        newFlags |= flag;
+      } else {
+        newFlags &= ~flag;
+      }
+    };
+    updateCombinedFlag(flutter::kModifierFlagShiftLeft, flutter::kModifierFlagShiftRight,
+                       NSEventModifierFlagShift);
+    updateCombinedFlag(flutter::kModifierFlagControlLeft, flutter::kModifierFlagControlRight,
+                       NSEventModifierFlagControl);
+    updateCombinedFlag(flutter::kModifierFlagAltLeft, flutter::kModifierFlagAltRight,
+                       NSEventModifierFlagOption);
+    updateCombinedFlag(flutter::kModifierFlagMetaLeft, flutter::kModifierFlagMetaRight,
+                       NSEventModifierFlagCommand);
+
+    NSEvent* event = [NSEvent keyEventWithType:NSEventTypeFlagsChanged
+                                      location:NSZeroPoint
+                                 modifierFlags:newFlags
+                                     timestamp:timestamp
+                                  windowNumber:0
+                                       context:nil
+                                    characters:@""
+                   charactersIgnoringModifiers:@""
+                                     isARepeat:NO
+                                       keyCode:keyCode];
+    [self handleEvent:event
+             callback:^(BOOL){
+             }];
+  };
+}
+
+- (void)syncModifiersIfNeeded:(NSEventModifierFlags)modifierFlags
+                    timestamp:(NSTimeInterval)timestamp {
+  modifierFlags = modifierFlags & ~0x100;
+  if (_previouslyPressedFlags == modifierFlags) {
+    return;
+  }
+
+  [flutter::modifierFlagToKeyCode
+      enumerateKeysAndObjectsUsingBlock:^(NSNumber* flag, NSNumber* keyCode, BOOL* stop) {
+        [self checkModifierFlag:[flag unsignedShortValue]
+                  forEventFlags:modifierFlags
+                        keyCode:[keyCode unsignedShortValue]
+                      timestamp:timestamp];
+      }];
+
+  // Caps lock is not included in the modifierFlagToKeyCode map.
+  [self checkModifierFlag:NSEventModifierFlagCapsLock
+            forEventFlags:modifierFlags
+                  keyCode:0x00000039  // kVK_CapsLock
+                timestamp:timestamp];
+
+  // At the end we should end up with the same modifier flags as the event.
+  FML_DCHECK(_previouslyPressedFlags == modifierFlags);
+}
+
 - (void)handleEvent:(NSEvent*)event callback:(FlutterAsyncKeyCallback)callback {
   // Remove the modifier bits that Flutter is not interested in.
   NSEventModifierFlags modifierFlags = event.modifierFlags & ~0x100;

diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyPrimaryResponder.h b/shell/platform/darwin/macos/framework/Source/FlutterKeyPrimaryResponder.h
@@ -24,6 +24,15 @@ typedef void (^FlutterAsyncKeyCallback)(BOOL handled);
 @required
 - (void)handleEvent:(nonnull NSEvent*)event callback:(nonnull FlutterAsyncKeyCallback)callback;
 
+/**
+ * Synchronize the modifier flags if necessary. The new modifier flag would usually come from mouse
+ * event and may be out of sync with current keyboard state if the modifier flags have changed while
+ * window was not key.
+ */
+@required
+- (void)syncModifiersIfNeeded:(NSEventModifierFlags)modifierFlags
+                    timestamp:(NSTimeInterval)timestamp;
+
 /* A map from macOS key code to logical keyboard.
  *
  * The map is assigned on initialization, and updated when the user changes

diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm
@@ -337,10 +337,9 @@ - (void)buildLayout {
 
 - (void)syncModifiersIfNeeded:(NSEventModifierFlags)modifierFlags
                     timestamp:(NSTimeInterval)timestamp {
-  // The embedder responder is the first element in _primaryResponders.
-  FlutterEmbedderKeyResponder* embedderResponder =
-      (FlutterEmbedderKeyResponder*)_primaryResponders[0];
-  [embedderResponder syncModifiersIfNeeded:modifierFlags timestamp:timestamp];
+  for (id<FlutterKeyPrimaryResponder> responder in _primaryResponders) {
+    [responder syncModifiersIfNeeded:modifierFlags timestamp:timestamp];
+  }
 }
 
 /**

diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm
@@ -957,13 +957,17 @@ - (bool)testMouseDownUpEventsSentToNextResponder {
 
 - (bool)testModifierKeysAreSynthesizedOnMouseMove {
   id engineMock = flutter::testing::CreateMockFlutterEngine(@"");
+  id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
+  OCMStub(  // NOLINT(google-objc-avoid-throwing-exception)
+      [engineMock binaryMessenger])
+      .andReturn(binaryMessengerMock);
+
   // Need to return a real renderer to allow view controller to load.
   FlutterRenderer* renderer_ = [[FlutterRenderer alloc] initWithFlutterEngine:engineMock];
   OCMStub([engineMock renderer]).andReturn(renderer_);
 
   // Capture calls to sendKeyEvent
-  __block NSMutableArray<KeyEventWrapper*>* events =
-      [[NSMutableArray<KeyEventWrapper*> alloc] init];
+  __block NSMutableArray<KeyEventWrapper*>* events = [NSMutableArray array];
   OCMStub([[engineMock ignoringNonObjectArgs] sendKeyEvent:FlutterKeyEvent {}
                                                   callback:nil
                                                   userData:nil])
@@ -973,6 +977,17 @@ - (bool)testModifierKeysAreSynthesizedOnMouseMove {
         [events addObject:[[KeyEventWrapper alloc] initWithEvent:event]];
       }));
 
+  __block NSMutableArray<NSDictionary*>* channelEvents = [NSMutableArray array];
+  OCMStub([binaryMessengerMock sendOnChannel:@"flutter/keyevent"
+                                     message:[OCMArg any]
+                                 binaryReply:[OCMArg any]])
+      .andDo((^(NSInvocation* invocation) {
+        NSData* data;
+        [invocation getArgument:&data atIndex:3];
+        id event = [[FlutterJSONMessageCodec sharedInstance] decode:data];
+        [channelEvents addObject:event];
+      }));
+
   FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock
                                                                                 nibName:@""
                                                                                  bundle:nil];
@@ -987,12 +1002,27 @@ - (bool)testModifierKeysAreSynthesizedOnMouseMove {
   // For each modifier key, check that key events are synthesized.
   for (NSNumber* keyCode in flutter::keyCodeToModifierFlag) {
     FlutterKeyEvent* event;
+    NSDictionary* channelEvent;
     NSNumber* logicalKey;
     NSNumber* physicalKey;
-    NSNumber* flag = flutter::keyCodeToModifierFlag[keyCode];
+    NSEventModifierFlags flag = [flutter::keyCodeToModifierFlag[keyCode] unsignedLongValue];
+
+    // Cocoa event always contain combined flags.
+    if (flag & (flutter::kModifierFlagShiftLeft | flutter::kModifierFlagShiftRight)) {
+      flag |= NSEventModifierFlagShift;
+    }
+    if (flag & (flutter::kModifierFlagControlLeft | flutter::kModifierFlagControlRight)) {
+      flag |= NSEventModifierFlagControl;
+    }
+    if (flag & (flutter::kModifierFlagAltLeft | flutter::kModifierFlagAltRight)) {
+      flag |= NSEventModifierFlagOption;
+    }
+    if (flag & (flutter::kModifierFlagMetaLeft | flutter::kModifierFlagMetaRight)) {
+      flag |= NSEventModifierFlagCommand;
+    }
 
     // Should synthesize down event.
-    NSEvent* mouseEvent = flutter::testing::CreateMouseEvent([flag unsignedLongValue]);
+    NSEvent* mouseEvent = flutter::testing::CreateMouseEvent(flag);
     [viewController mouseMoved:mouseEvent];
     EXPECT_EQ([events count], 1u);
     event = events[0].data;
@@ -1003,6 +1033,11 @@ - (bool)testModifierKeysAreSynthesizedOnMouseMove {
     EXPECT_EQ(event->physical, physicalKey.unsignedLongLongValue);
     EXPECT_EQ(event->synthesized, true);
 
+    channelEvent = channelEvents[0];
+    EXPECT_TRUE([channelEvent[@"type"] isEqual:@"keydown"]);
+    EXPECT_TRUE([channelEvent[@"keyCode"] isEqual:keyCode]);
+    EXPECT_TRUE([channelEvent[@"modifiers"] isEqual:@(flag)]);
+
     // Should synthesize up event.
     mouseEvent = flutter::testing::CreateMouseEvent(0x00);
     [viewController mouseMoved:mouseEvent];
@@ -1015,7 +1050,13 @@ - (bool)testModifierKeysAreSynthesizedOnMouseMove {
     EXPECT_EQ(event->physical, physicalKey.unsignedLongLongValue);
     EXPECT_EQ(event->synthesized, true);
 
+    channelEvent = channelEvents[1];
+    EXPECT_TRUE([channelEvent[@"type"] isEqual:@"keyup"]);
+    EXPECT_TRUE([channelEvent[@"keyCode"] isEqual:keyCode]);
+    EXPECT_TRUE([channelEvent[@"modifiers"] isEqual:@(0)]);
+
     [events removeAllObjects];
+    [channelEvents removeAllObjects];
   };
 
   return true;