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 182 matching lines @@
   // This is a special case for which the complete string should should be
   // returned. NSTextView also follows this, though the same is not mentioned in
   // NSTextInputClient documentation.
   if (!requested_range.IsValid())
     *actual_range = text_range;
 
   client->GetTextFromRange(*actual_range, &substring);
   return substring;
 }
 
-// Returns a character event corresponding to |event|. |event| must be a
-// character event itself.
-ui::KeyEvent GetCharacterEventFromNSEvent(NSEvent* event) {
-  DCHECK([event type] == NSKeyDown || [event type] == NSKeyUp);
-  DCHECK_EQ(1u, [[event characters] length]);
-
-  // [NSEvent characters] already considers the pressed key modifiers. Hence
-  // send ui::EF_NONE as the key modifier to the KeyEvent constructor.
-  // E.g. For Alt+S, [NSEvent characters] is 'ß' and not 'S'.
-  return ui::KeyEvent([[event characters] characterAtIndex:0],
-                      ui::KeyboardCodeFromNSEvent(event), ui::EF_NONE);
-}
-
 NSAttributedString* GetAttributedString(
     const gfx::DecoratedText& decorated_text) {
   base::scoped_nsobject<NSMutableAttributedString> str(
       [[NSMutableAttributedString alloc]
           initWithString:base::SysUTF16ToNSString(decorated_text.text)]);
   [str beginEditing];
 
   NSValue* const line_style =
       @(NSUnderlineStyleSingle | NSUnderlinePatternSolid);
 
@@ skipping 229 matching lines @@
     return;
 
   DCHECK(event);
   if (DispatchEventToMenu([self activeMenuController], event))
     return;
 
   hostedView_->GetWidget()->GetInputMethod()->DispatchKeyEvent(event);
 }
 
 - (BOOL)handleUnhandledKeyDownAsKeyEvent {
-  if (!keyDownEvent_)
+  if (!hasUnhandledKeyDownEvent_)
     return NO;
 
   ui::KeyEvent event(keyDownEvent_);
   [self handleKeyEvent:&event];
-  keyDownEvent_ = nil;
+  hasUnhandledKeyDownEvent_ = NO;
   return event.handled();
 }
 
 - (void)handleAction:(ui::TextEditCommand)command
              keyCode:(ui::KeyboardCode)keyCode
              domCode:(ui::DomCode)domCode
           eventFlags:(int)eventFlags {
   if (!hostedView_)
     return;
 
@@ skipping 18 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 press 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.
+  // Note we create a custom character ui::KeyEvent (and not use the
+  // ui::KeyEvent(NSEvent*) constructor) since we can't just rely on the event
+  // key code to get the actual characters from the ui::KeyEvent. This for
+  // example is necessary for menu mnemonic selection of non-latin text.
+
+  // Don't generate a key event when there is active composition text. These key
+  // down events should be consumed by the IME and not reach the Views layer.
+  // For example, on pressing Return to commit composition text, if we passed a
+  // synthetic key event to the View hierarchy, it will have the effect of
+  // performing the default action on the current dialog. We do not want this.
+
+  // Also note that a single key down event can cause multiple
+  // insertText:replacementRange: action messages. Example, on pressing Alt+e,
+  // the accent (´) character is composed via setMarkedText:. Now on pressing
+  // the character 'r', two insertText:replacementRange: action messages are
+  // generated with the text value of accent (´) and 'r' respectively. The key
+  // down event will have characters field of length 2. The first of these
+  // insertText messages won't generate a KeyEvent since there'll be active
+  // marked text. However, a KeyEvent will be generated corresponding to 'r'.
+
+  // Currently there seems to be no use case to pass non-character events routed
+  // from insertText: handlers to the View hierarchy.
+  if (isCharacterEvent && ![self hasMarkedText]) {
+    ui::KeyEvent charEvent([text characterAtIndex:0],
+                           ui::KeyboardCodeFromNSEvent(keyDownEvent_),
+                           ui::EF_NONE);
+    [self handleKeyEvent:&charEvent];
+    hasUnhandledKeyDownEvent_ = NO;
+  }
 
   // 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.
+    // ui::KeyEvent since for composed text, [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
-  //   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.
+    hasUnhandledKeyDownEvent_ = NO;
   }
 }
 
 - (views::DragDropClientMac*)dragDropClient {
   views::BridgedNativeWidget* bridge =
       views::NativeWidgetMac::GetBridgeForNativeWindow([self window]);
   return bridge ? bridge->drag_drop_client() : nullptr;
 }
 
 - (void)undo:(id)sender {
@@ skipping 238 matching lines @@
   // correctly, etc.
   // (However, there are still some keys that Cocoa swallows, e.g. the key
   // equivalent that Cocoa uses for toggling the input language. In this case,
   // that's actually a good thing, though -- see http://crbug.com/26115 .)
   return YES;
 }
 
 - (void)keyDown:(NSEvent*)theEvent {
   // Convert the event into an action message, according to OSX key mappings.
   keyDownEvent_ = theEvent;
+  hasUnhandledKeyDownEvent_ = YES;
   [self interpretKeyEvents:@[ theEvent ]];
 
   // If |keyDownEvent_| wasn't cleared during -interpretKeyEvents:, it wasn't
   // handled. Give Widget accelerators a chance to handle it.
   [self handleUnhandledKeyDownAsKeyEvent];
-  DCHECK(!keyDownEvent_);
+  DCHECK(!hasUnhandledKeyDownEvent_);
+  keyDownEvent_ = nil;
 }
 
 - (void)keyUp:(NSEvent*)theEvent {
   ui::KeyEvent event(theEvent);
   [self handleKeyEvent:&event];
 }
 
 - (void)scrollWheel:(NSEvent*)theEvent {
   if (!hostedView_)
     return;
@@ skipping 503 matching lines @@
 }
 
 - (void)doCommandBySelector:(SEL)selector {
   // Like the renderer, handle insert action messages as a regular key dispatch.
   // This ensures, e.g., insertTab correctly changes focus between fields.
   if (keyDownEvent_ && [NSStringFromSelector(selector) hasPrefix:@"insert"])
     return;  // Handle in -keyDown:.
 
   if ([self respondsToSelector:selector]) {
     [self performSelector:selector withObject:nil];
-    keyDownEvent_ = nil;
+    hasUnhandledKeyDownEvent_ = NO;
     return;
   }
 
   // For events that AppKit sends via doCommandBySelector:, first attempt to
   // handle as a Widget accelerator. Forward along the responder chain only if
   // the Widget doesn't handle it.
   if (![self handleUnhandledKeyDownAsKeyEvent])
     [[self nextResponder] doCommandBySelector:selector];
 }
 
@@ skipping 58 matching lines @@
   // Add a black underline with a transparent background to the composition
   // text. TODO(karandeepb): On Cocoa textfields, the target clause of the
   // composition has a thick underlines. The composition text also has
   // discontinous underlines for different clauses. This is also supported in
   // the Chrome renderer. Add code to extract underlines from |text| once our
   // render text implementation supports thick underlines and discontinous
   // underlines for consecutive characters. See http://crbug.com/612675.
   composition.underlines.push_back(ui::CompositionUnderline(
       0, [text length], SK_ColorBLACK, false, SK_ColorTRANSPARENT));
   textInputClient_->SetCompositionText(composition);
-  keyDownEvent_ = nil;  // Handled.
+  hasUnhandledKeyDownEvent_ = NO;
 }
 
 - (void)unmarkText {
   if (!textInputClient_)
     return;
 
   textInputClient_->ConfirmCompositionText();
-  keyDownEvent_ = nil;  // Handled.
+  hasUnhandledKeyDownEvent_ = NO;
 }
 
 - (NSArray*)validAttributesForMarkedText {
   return @[];
 }
 
 // NSUserInterfaceValidations protocol implementation.
 
 - (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item {
   ui::TextEditCommand command = GetTextEditCommandForMenuAction([item action]);
@@ skipping 46 matching lines @@
   return [hostedView_->GetNativeViewAccessible() accessibilityHitTest:point];
 }
 
 - (id)accessibilityFocusedUIElement {
   if (!hostedView_)
     return nil;
   return [hostedView_->GetNativeViewAccessible() accessibilityFocusedUIElement];
 }
 
 @end