Mac: Implement firstRectForCharacterRange:actualRange in BridgedContentView.

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 2167561368c38f1b8a637d99c1b1daa022faae15..c01552f10a710e93b0eae336c74f8a5b247c8cda 100644
--- a/ui/views/cocoa/bridged_content_view.mm
+++ b/ui/views/cocoa/bridged_content_view.mm
@@ -16,6 +16,7 @@
 #import "ui/events/keycodes/keyboard_code_conversion_mac.h"
 #include "ui/gfx/canvas_paint_mac.h"
 #include "ui/gfx/geometry/rect.h"
+#include "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"
@@ -56,6 +57,91 @@ bool DispatchEventToMenu(views::Widget* widget, ui::KeyboardCode key_code) {
   return false;
 }
 
+// Helper function for firstRectForCharacterRange:actualRange defined below.

[tapted, 2015/12/30 06:48:52]

This comment doesn't add much beyond the function name. It should be something
like "Query |client| for the screen rectangle encompassing |requested_range|.
|actual_range| is set to the subset of the requested range that can be matched
by |client|. If none of the range fits, returns the bounds of the input caret."

Maybe more than that if there are other special behaviours you've implemented.
You can also cite another function if this is "Standard" behaviour documented
elsewhere that's just being implemented here for mac.

[karandeepb, 2015/12/31 02:42:43]

Done.

+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 = gfx::Rect();

[tapted, 2015/12/30 06:48:52]

nit: no need for the ` = gfx::Rect()` - just use default constructor

[karandeepb, 2015/12/31 02:42:43]

Done.

+  *actual_range = gfx::Range::InvalidRange();
+  if (!client)
+    return default_rect;
+
+  // If possible, modify default return values to correspond to caret position.
+  gfx::Range selection_range;
+  if (client->GetSelectionRange(&selection_range)) {
+    default_rect = client->GetCaretBounds();
+    default_rect.set_width(0);
+    // Caret bounds correspond to end index of selection_range.
+    *actual_range = gfx::Range(selection_range.end());
+  }
+
+  if (!client->HasCompositionText())
+    return default_rect;
+
+  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().
+  gfx::Range relative_range(requested_range.start() - composition_range.start(),

[tapted, 2015/12/30 06:48:52]

nit: declare const

[karandeepb, 2015/12/31 02:42:43]

Done.

+                            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;
+  size_t composition_count = composition_range.length();

[tapted, 2015/12/30 06:48:52]

nit: declare const

[karandeepb, 2015/12/31 02:42:43]

Done.

+
+  // In the case where relative_range is [composition_count, composition_count],
+  // use the bounds for the last compositioned character.

[tapted, 2015/12/30 06:48:52]

Is using the last character bounds better than using the caret bounds in this
case? (or is that done by something else already?) shuchen might know the right
answer :)

[karandeepb, 2015/12/31 02:42:43]

This is how  RenderWidgetHostViewMac::GetCachedFirstRectForCharacterRange(...)
handles this -
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re....
Also, I think it's fairly intuitive. Since the query range is
[composition_count, composition_count], we are just returning an empty rectangle
to the right of the last compositioned character(i.e. character at index
composition_count-1).

+  const bool request_is_composition_end =
+      relative_range.start() == composition_count;
+  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());
+    }
+    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.
+  gfx::Rect current_rect;

[tapted, 2015/12/30 06:48:52]

move declaration into loop body

[karandeepb, 2015/12/31 02:42:43]

Done.

+  for (size_t i = relative_range.start() + 1; i < relative_range.end(); i++) {
+    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;
+}
+
 }  // namespace
 
 @interface BridgedContentView ()
@@ -591,9 +677,13 @@ bool DispatchEventToMenu(views::Widget* widget, ui::KeyboardCode key_code) {
 }
 
 - (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 {