MacViews: Implement text input.

ui/views/cocoa/bridged_content_view.mm

Index: ui/views/cocoa/bridged_content_view.mm
diff --git a/ui/views/cocoa/bridged_content_view.mm b/ui/views/cocoa/bridged_content_view.mm
index 6be51fd9f5df3ad587c370c76d5a82fe0e12ee72..beec56a7c8ba85ba9da98ead538c06c8f58acb7c 100644
--- a/ui/views/cocoa/bridged_content_view.mm
+++ b/ui/views/cocoa/bridged_content_view.mm
@@ -4,10 +4,15 @@
 
 #import "ui/views/cocoa/bridged_content_view.h"
 
+#include "base/strings/sys_string_conversions.h"
+#include "grit/ui_strings.h"
+#include "ui/base/ime/text_input_client.h"
 #include "ui/gfx/mac/point_utils.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/canvas_paint_mac.h"
+#include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/mac/point_utils.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
@@ -41,6 +46,10 @@
   hostedView_ = NULL;
 }
 
+- (void)setTextInputClient:(ui::TextInputClient*)textInputClient {
+  textInputClient_ = textInputClient;

[tapted, 2014/06/17 13:23:56]

Does toolkit-views guarantee to send OnDidChangeFocus away from the View when
it's destroyed and takes its TextInputClient away with it?

[Andre, 2014/06/18 21:48:34]

Yes, that should be handled through Widget::ViewHierarchyChanged and
FocusManager::ViewRemoved.

+}
+
 // BridgedContentView private implementation.
 
 - (void)handleUntranslatedKeyEvent:(NSEvent*)theEvent {
@@ -59,8 +68,17 @@
   hostedView_->GetWidget()->OnMouseEvent(&event);
 }
 
+- (void)doCommandByID:(int)cmd_id {

[tapted, 2014/06/17 13:23:56]

This should be declared in BridgedContentView () with a comment

and, nit: cmd_id -> commandId (no abbreviations). Or, just `id` is probably fine
too. resourceId might be another option, to make it clear that it's an ID coming
from grit

[Andre, 2014/06/18 21:48:34]

Done.

+  if (textInputClient_ && textInputClient_->IsEditingCommandEnabled(cmd_id))
+    textInputClient_->ExecuteEditingCommand(cmd_id);
+}
+
 // NSView implementation.
 
+- (BOOL)acceptsFirstResponder {
+  return YES;
+}
+
 - (void)setFrame:(NSRect)newFrame {
   [super setFrame:newFrame];
   if (!hostedView_)
@@ -78,7 +96,10 @@
 }
 
 - (void)keyDown:(NSEvent*)theEvent {
-  [self handleUntranslatedKeyEvent:theEvent];
+  if (textInputClient_)
+    [self interpretKeyEvents:@[ theEvent ]];

[tapted, 2014/06/17 13:23:56]

Do you think interpretKeyEvents will be needed for some cases where
textInputClient_ is NULL? I actually couldn't decide how to tie in
interpretKeyEvents when I added keyUp/keyDown. Is your approach similar to how
the renderer does it?

[Andre, 2014/06/18 21:48:35]

If textInputClient_ is NULL, I think we should forward the key event to the next
responder by calling super. It will give other responders in the chain to
process the
event, and the event is not handled the system will emit a beep.

[tapted, 2014/06/19 01:29:15]

yep - lg for now. For things like menus/lists we'll want arrow keys for
navigation - i don't think they will ahve a textInputClient, but we should be
going through interpretKeyEvents for those anyway to ensure we pick up things a
user has reconfigured.

+  else
+    [self handleUntranslatedKeyEvent:theEvent];
 }
 
 - (void)keyUp:(NSEvent*)theEvent {
@@ -97,4 +118,105 @@
   [self handleMouseEvent:theEvent];
 }
 
+- (void)deleteBackward:(id)sender {
+  [self doCommandByID:IDS_DELETE_BACKWARD];
+}
+
+- (void)deleteForward:(id)sender {
+  [self doCommandByID:IDS_DELETE_FORWARD];
+}
+
+- (void)moveLeft:(id)sender {
+  [self doCommandByID:IDS_MOVE_LEFT];
+}
+
+- (void)moveRight:(id)sender {
+  [self doCommandByID:IDS_MOVE_RIGHT];
+}
+
+// NSTextInputClient protocol implementation.
+
+- (NSAttributedString*)attributedSubstringForProposedRange:(NSRange)range

[tapted, 2014/06/17 13:23:56]

ugh longmethodnames. is this what clang-format does? I'd probably try breaking
after the return type and aligning colons

[Andre, 2014/06/18 21:48:34]

Done.

+    actualRange:(NSRangePointer)actualRange {
+  base::string16 text;

[tapted, 2014/06/17 13:23:56]

is there a better name for `text` - `substring` maybe?

[Andre, 2014/06/18 21:48:34]

Done.

+  if (textInputClient_) {
+    gfx::Range textRange;
+    textInputClient_->GetTextRange(&textRange);
+    gfx::Range subrange = textRange.Intersect(gfx::Range(range));
+    textInputClient_->GetTextFromRange(subrange, &text);
+    if (actualRange)
+      *actualRange = subrange.ToNSRange();
+  }
+  return [[[NSAttributedString alloc]
+      initWithString:base::SysUTF16ToNSString(text)] autorelease];
+}
+
+- (NSUInteger)characterIndexForPoint:(NSPoint)aPoint {
+  NOTIMPLEMENTED();
+  return 0;
+}
+
+- (void)doCommandBySelector:(SEL)selector {

[tapted, 2014/06/17 13:23:56]

Should be under `// NSResponder implementation.`. (but why was this needed - I
thought the AppKit NSResponder dispatcher took care of this

[Andre, 2014/06/18 21:48:34]

doCommandBySelector is part of both NSResponder and NSTextInputClient.
In this case, we are implementing the NSTextInputClient one so I don't think it
should
be under NSResponder implementation.
I think you're right that NSResponder's default implementation probably takes
care of
this, but this is a required method in NSTextInputClient protocol, and the docs
state
that it should not call super.
It probably should forward unhandled selectors to nextResponder though, so I've
added code for that.

[tapted, 2014/06/19 01:29:15]

It's still overriding a method on NSResponder (whereas it's just conforming to a
protocol on NSTextInputClient). I guess either is fine.

+  if ([self respondsToSelector:selector])
+    [self performSelector:selector withObject:nil];
+}
+
+- (NSRect)firstRectForCharacterRange:(NSRange)range
+                         actualRange:(NSRangePointer)actualRange {
+  // FIXME(andresantoso): ui::TextInputClient does not expose this.

[tapted, 2014/06/17 13:23:56]

I don't typically see FIXME comments. TODO is usually fine though

[Andre, 2014/06/18 21:48:34]

Yep, that was a temporary experiment.
I've changed it to NOTIMPLEMENTED for now.

+  return textInputClient_
+             ? gfx::ScreenRectToNSRect(textInputClient_->GetCaretBounds())
+             : NSZeroRect;
+}
+
+- (BOOL)hasMarkedText {
+  return textInputClient_ && textInputClient_->HasCompositionText();
+}
+
+- (void)insertText:(id)text replacementRange:(NSRange)replacementRange {

[tapted, 2014/06/17 13:23:56]

nit: I think method signatures "always" linebreak between arguments in chromium
objc

[Andre, 2014/06/18 21:48:34]

Hmm, I don't see that in objcstyle, and cocoa code in chrome/browser does not
seem
to do that. Where do you see the always linebreak convention?

[tapted, 2014/06/19 01:29:15]

Oh wow - I've just always done it - but you're right. I guess objC argument
names are usually so long it hardly ever comes up

+  if ([text isKindOfClass:[NSAttributedString class]])

[tapted, 2014/06/17 13:23:56]

Before this, you should do

if (!textInputClient_)
  return;

[Andre, 2014/06/18 21:48:34]

Done.

[tapted, 2014/06/19 01:29:15]

`if (!textInputClient_)` should be the first line of the function - it just
keeps the nesting down to a minimum.

[Andre, 2014/06/19 23:20:25]

Done.

+    text = [text string];
+  if (textInputClient_) {
+    textInputClient_->DeleteRange(gfx::Range(replacementRange));
+    textInputClient_->InsertText(base::SysNSStringToUTF16(text));
+  }
+}
+
+- (NSRange)markedRange {
+  gfx::Range range;

[tapted, 2014/06/17 13:23:56]

if (!textInputClient_)
  return NSMakeRange(0, 0);

more below.

[Andre, 2014/06/18 21:48:34]

Done.

+  if (textInputClient_)
+    textInputClient_->GetCompositionTextRange(&range);
+  return range.ToNSRange();
+}
+
+- (NSRange)selectedRange {
+  gfx::Range range;
+  if (textInputClient_)
+    textInputClient_->GetSelectionRange(&range);
+  return range.ToNSRange();
+}
+
+- (void)setMarkedText:(id)text
+        selectedRange:(NSRange)selectedRange
+     replacementRange:(NSRange)replacementRange {
+  if ([text isKindOfClass:[NSAttributedString class]])
+    text = [text string];
+
+  if (textInputClient_) {
+    ui::CompositionText composition;
+    composition.text = base::SysNSStringToUTF16(text);
+    composition.selection = gfx::Range(selectedRange);
+    textInputClient_->SetCompositionText(composition);
+  }
+}
+
+- (void)unmarkText {
+  if (textInputClient_)
+    textInputClient_->ConfirmCompositionText();
+}
+
+- (NSArray*)validAttributesForMarkedText {
+  return @[];
+}
+
 @end