MacViews: Correctly handle character events when there's an active TextInputClient.

ui/views/cocoa/bridged_content_view.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ui/views/cocoa/bridged_content_view.h"
 
 #include "base/logging.h"
 #import "base/mac/mac_util.h"
 #import "base/mac/scoped_nsobject.h"
 #import "base/mac/sdk_forward_declarations.h"
@@ skipping 489 matching lines @@
 
 - (void)insertTextInternal:(id)text {
   if (!hostedView_)
     return;
 
   if ([text isKindOfClass:[NSAttributedString class]])
     text = [text string];
 
   bool isCharacterEvent = keyDownEvent_ && [text length] == 1;
 
+  // Pass the character event to the View hierarchy. Cases this handles (non-
+  // exhaustive)-
+  //    - Space key presses on controls. Unlike Tab and newline which have
+  //      corresponding action messages, an insertText: message is generated for
+  //      the Space key (insertText:replacementRange: when there's an active
+  //      input context).
+  //    - Menu mnemonic selection.
+  // Currently there seems to be no use case to pass non-character events
+  // routed from insertText: handlers, to the View hierarchy.
+  if (isCharacterEvent) {
+    ui::KeyEvent charEvent = GetCharacterEventFromNSEvent(keyDownEvent_);
+    [self handleKeyEvent:&charEvent];
+    keyDownEvent_ = nil;  // Handled.
+  }
+
   // Forward the |text| to |textInputClient_| if no menu is active.
   if (textInputClient_ && ![self activeMenuController]) {
     // If a single character is inserted by keyDown's call to
     // interpretKeyEvents: then use InsertChar() to allow editing events to be
     // merged. We use ui::VKEY_UNKNOWN as the key code since it's not feasible
     // to determine the correct key code for each unicode character. Also a
     // correct keycode is not needed in the current context. Send ui::EF_NONE as
     // the key modifier since |text| already accounts for the pressed key
     // modifiers.
 
     // Also, note we don't use |keyDownEvent_| to generate the synthetic
     // ui::KeyEvent since for text inserted using an IME, [keyDownEvent_
     // characters] might not be the same as |text|. This is because
     // |keyDownEvent_| will correspond to the event that caused the composition
     // text to be confirmed, say, Return key press.
     if (isCharacterEvent) {
       textInputClient_->InsertChar(ui::KeyEvent([text characterAtIndex:0],
                                                 ui::VKEY_UNKNOWN, ui::EF_NONE));
     } else {
       textInputClient_->InsertText(base::SysNSStringToUTF16(text));
     }
-
-    keyDownEvent_ = nil;  // Handled.
-    return;
-  }
-
-  // Only handle the case where no. of characters is 1. Cases not handled (not
-  // an exhaustive list):
-  // - |text| contains a unicode surrogate pair, i.e. a single grapheme which
-  //   requires two 16 bit characters. Currently Views menu only supports

[karandeepb, 2017/01/11 06:00:58]

Also, modified this comment. 

-  //   mnemonics using a single 16 bit character, so it is ok to ignore this
-  //   case.
-  // - Programmatically created events.
-  // - Input from IME. But this case should not occur since inputContext is
-  //   nil.
-  if (isCharacterEvent) {
-    ui::KeyEvent charEvent = GetCharacterEventFromNSEvent(keyDownEvent_);
-    [self handleKeyEvent:&charEvent];
     keyDownEvent_ = nil;  // Handled.
   }
 }
 
 - (views::DragDropClientMac*)dragDropClient {
   views::BridgedNativeWidget* bridge =
       views::NativeWidgetMac::GetBridgeForNativeWindow([self window]);
   return bridge ? bridge->drag_drop_client() : nullptr;
 }
 
@@ skipping 922 matching lines @@
   return [hostedView_->GetNativeViewAccessible() accessibilityHitTest:point];
 }
 
 - (id)accessibilityFocusedUIElement {
   if (!hostedView_)
     return nil;
   return [hostedView_->GetNativeViewAccessible() accessibilityFocusedUIElement];
 }
 
 @end