Mac: Implement firstRectForCharacterRange:actualRange in BridgedContentView.

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/scoped_nsobject.h"
 #include "base/strings/sys_string_conversions.h"
 #include "skia/ext/skia_utils_mac.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/compositor/canvas_painter.h"
 #import "ui/events/cocoa/cocoa_event_utils.h"
 #include "ui/events/keycodes/dom/dom_code.h"
 #import "ui/events/keycodes/keyboard_code_conversion_mac.h"
 #include "ui/gfx/canvas_paint_mac.h"
 #include "ui/gfx/geometry/rect.h"
+#import "ui/gfx/mac/coordinate_conversion.h"
 #include "ui/strings/grit/ui_strings.h"
 #include "ui/views/controls/menu/menu_config.h"
 #include "ui/views/controls/menu/menu_controller.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 using views::MenuController;
 
 namespace {
 
@@ skipping 20 matching lines @@
 bool DispatchEventToMenu(views::Widget* widget, ui::KeyboardCode key_code) {
   MenuController* menuController = MenuController::GetActiveInstance();
   if (menuController && menuController->owner() == widget) {
     if (menuController->OnWillDispatchKeyEvent(0, key_code) ==
         ui::POST_DISPATCH_NONE)
       return true;
   }
   return false;
 }
 
+// Returns the boundary rectangle for composition characters in the
+// |requested_range|. Sets |actual_range| corresponding to the returned
+// rectangle. For cases, where there is no composition text or the
+// |requested_range| lies outside the composition range, a zero width rectangle
+// corresponding to the caret bounds is returned. Logic used is similar to
+// RenderWidgetHostViewMac::GetCachedFirstRectForCharacterRange(...).
+gfx::Rect GetFirstRectForRangeHelper(const ui::TextInputClient* client,
+                                     const gfx::Range& requested_range,
+                                     gfx::Range* actual_range) {
+  // NSRange doesn't support reversed ranges.
+  DCHECK(!requested_range.is_reversed());
+  DCHECK(actual_range);
+
+  // Set up default return values, to be returned in case of unusual cases.
+  gfx::Rect default_rect;
+  *actual_range = gfx::Range::InvalidRange();
+  if (!client)
+    return default_rect;
+
+  default_rect = client->GetCaretBounds();
+  default_rect.set_width(0);

[Shu Chen, 2016/01/08 04:18:53]

Why zero?

[karandeepb, 2016/01/12 05:43:52]

From what I have seen, IME clients query with an empty range when there is no
composition text. The documentation at
https://developer.apple.com/library/mac/documentation/Cocoa/Reference/NSTextI...:
says to return a 0 width rectangle coinciding with the insertion point when the
width of requested range is 0. Hence I thought it to be a good default value.

[Shu Chen, 2016/01/12 05:54:35]

Acknowledged.

+
+  // If possible, modify actual_range to correspond to caret position.
+  gfx::Range selection_range;
+  if (client->GetSelectionRange(&selection_range)) {
+    // Caret bounds correspond to end index of selection_range.
+    *actual_range = gfx::Range(selection_range.end());
+  }
+
+  if (!client->HasCompositionText())
+    return default_rect;

[Shu Chen, 2016/01/08 04:18:53]

Should return the selection bounds (if valid) I think.

[karandeepb, 2016/01/12 05:43:52]

Do you mean the bounds corresponding to the 'whole' area of selected text or the
caret bounds with the width not set to 0?  The spec of GetCaretBounds() in
textInputClient says that it returns the selection bounds if there is a
selection. However, in practice, it returns the caret bounds(and not the bounds
corresponding to the full selection).

[Shu Chen, 2016/01/12 05:54:35]

Ah, thanks for the clarification. That should be a separated bug to fix.

+
+  gfx::Range composition_range;
+  if (!client->GetCompositionTextRange(&composition_range))
+    return default_rect;
+
+  DCHECK(!composition_range.is_reversed());
+
+  if (!composition_range.Contains(requested_range))
+    return default_rect;
+
+  // Range relative to composition_range.start().
+  const gfx::Range relative_range(
+      requested_range.start() - composition_range.start(),
+      requested_range.end() - composition_range.start());
+
+  // Pick the first character's bounds as the initial rectangle, then grow it to
+  // the full |requested_range| if possible.
+  gfx::Rect union_rect;
+  const size_t composition_count = composition_range.length();
+
+  // In the case where relative_range is [composition_count, composition_count],
+  // use the bounds for the last compositioned character.
+  const bool request_is_composition_end =
+      relative_range.start() == composition_count;
+  const size_t first_index = request_is_composition_end
+                                 ? composition_count - 1
+                                 : relative_range.start();
+  if (!client->GetCompositionCharacterBounds(first_index, &union_rect))
+    return default_rect;
+
+  // If relative_range is empty, return a zero width rectangle corresponding to
+  // the relative_range.
+  if (relative_range.is_empty()) {
+    if (request_is_composition_end) {
+      // In case of an empty requested range at end of composition, return the
+      // rectangle to the right of the last compositioned character.
+      union_rect.set_origin(union_rect.top_right());

[Shu Chen, 2016/01/08 04:18:53]

top_right() may not be correct for RTL text.

[karandeepb, 2016/01/12 05:43:52]

Done.

+    }
+    union_rect.set_width(0);
+    *actual_range = requested_range;
+    return union_rect;
+  }
+
+  // Toolkit-views textfields are always single-line, so no need to check for
+  // line breaks.
+
+  // Return the union rectangle of the character bounds within the
+  // relative_range.
+  for (size_t i = relative_range.start() + 1; i < relative_range.end(); i++) {
+    gfx::Rect current_rect;
+    if (client->GetCompositionCharacterBounds(i, &current_rect)) {
+      union_rect.Union(current_rect);
+    } else {
+      *actual_range =
+          gfx::Range(requested_range.start(), i + composition_range.start());
+      return union_rect;
+    }
+  }
+  *actual_range = requested_range;
+  return union_rect;

[Shu Chen, 2016/01/08 04:18:53]

(optional) can you please make the code conciser? e.g. replace line 102~150:

size_t from = requested_range.start() - composition_range.start();
size_t to = requested_range.end() - composition_range.start();
gfx::Rect union_rect;
if (from == composition_range.length()) {
  if (!client->GetCompositionCharacterBounds(from - 1, &union_rect))
    return default_rect;
  union_rect.set_origin(union_rect.top_right());
  union_rect.set_width(0);
} else {
  for (size_t i = from; i < to; i++) {
    gfx::Rect current_rect;
    if (!client->GetCompositionCharacterBounds(i, &current_rect)) {
      *actual_range = ...;
      return union_rect;
    }
    union_rect.Union(current_rect);
  }
}
*actual_range = requested_range;
return union_rect;

[karandeepb, 2016/01/12 05:43:52]

This does not handle the case where from==to but from is inside the composition
range. This will return the empty rectangle but the current code returns a 0
width rectangle corresponding to from. Taking care of it, should result in code
similar to the current one. I have tried to make the code a bit concise. Apart
from that, most of the space is occupied by comments. Let me know if any of them
seem redundant to you.

[Shu Chen, 2016/01/12 05:54:35]

Acknowledged.

+}
+
 }  // namespace
 
 @interface BridgedContentView ()
 
 // Translates keycodes and modifiers on |theEvent| to ui::KeyEvents and passes
 // the event to the InputMethod for dispatch.
 - (void)handleKeyEvent:(NSEvent*)theEvent;
 
 // Handles an NSResponder Action Message by mapping it to a corresponding text
 // editing command from ui_strings.grd and, when not being sent to a
@@ skipping 515 matching lines @@
     return;
   }
 
   if ([self respondsToSelector:selector])
     [self performSelector:selector withObject:nil];
   else
     [[self nextResponder] doCommandBySelector:selector];
 }
 
 - (NSRect)firstRectForCharacterRange:(NSRange)range
-                         actualRange:(NSRangePointer)actualRange {
-  NOTIMPLEMENTED();
-  return NSZeroRect;
+                         actualRange:(NSRangePointer)actualNSRange {
+  gfx::Range actualRange;
+  gfx::Rect rect = GetFirstRectForRangeHelper(textInputClient_,
+                                              gfx::Range(range), &actualRange);
+  if (actualNSRange)
+    *actualNSRange = actualRange.ToNSRange();
+  return gfx::ScreenRectToNSRect(rect);
 }
 
 - (BOOL)hasMarkedText {
   return textInputClient_ && textInputClient_->HasCompositionText();
 }
 
 - (void)insertText:(id)text replacementRange:(NSRange)replacementRange {
   if (!hostedView_)
     return;
 
@@ skipping 115 matching lines @@
   }
 
   return [super accessibilityAttributeValue:attribute];
 }
 
 - (id)accessibilityHitTest:(NSPoint)point {
   return [hostedView_->GetNativeViewAccessible() accessibilityHitTest:point];
 }
 
 @end