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"
+#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"
 #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;
 }
 
+// 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 ()
 
 // 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