fix know issues in RenderText

ui/gfx/render_text.cc

 // Copyright (c) 2011 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.
 
 #include "ui/gfx/render_text.h"
 
 #include <algorithm>
 
 #include "base/i18n/break_iterator.h"
 #include "base/logging.h"
@@ skipping 229 matching lines @@
   // The current model only supports caret positions at valid character indices.
   if (text_length == 0) {
     sel.set_caret_pos(0);
     sel.set_caret_placement(SelectionModel::LEADING);
   } else if (sel.caret_pos() >= text_length) {
     SelectionModel end = GetTextDirection() == base::i18n::RIGHT_TO_LEFT ?
         LeftEndSelectionModel() : RightEndSelectionModel();
     sel.set_caret_pos(end.caret_pos());
     sel.set_caret_placement(end.caret_placement());
   }
+
+  if (!IsCursorablePosition(sel.selection_start()) ||
+      !IsCursorablePosition(sel.selection_end()) ||
+      !IsCursorablePosition(sel.caret_pos()))
+    return false;
+
   bool changed = !sel.Equals(selection_model_);
   SetSelectionModel(sel);
   return changed;
 }
 
 bool RenderText::MoveCursorTo(const Point& point, bool select) {
   SelectionModel selection = FindCursorPosition(point);
   if (select)
     selection.set_selection_start(GetSelectionStart());
   return MoveCursorTo(selection);
@@ skipping 14 matching lines @@
   sel.set_selection_start(GetCursorPosition());
   SetSelectionModel(sel);
 }
 
 void RenderText::SelectAll() {
   SelectionModel sel(RightEndSelectionModel());
   sel.set_selection_start(LeftEndSelectionModel().selection_start());
   SetSelectionModel(sel);
 }
 
-// TODO(xji): it does not work for languages do not use space as word breaker,
-// such as Chinese. Should use BreakIterator.
 void RenderText::SelectWord() {
-  size_t selection_start = GetSelectionStart();
   size_t cursor_position = GetCursorPosition();
-  // First we setup selection_start_ and selection_end_. There are so many cases
-  // because we try to emulate what select-word looks like in a gtk textfield.
-  // See associated testcase for different cases.
-  if (cursor_position > 0 && cursor_position < text().length()) {
-    if (u_isalnum(text()[cursor_position])) {
-      selection_start = cursor_position;
-      cursor_position++;
-    } else
-      selection_start = cursor_position - 1;
-  } else if (cursor_position == 0) {
-    selection_start = cursor_position;
-    if (text().length() > 0)
-      cursor_position++;
-  } else {
-    selection_start = cursor_position - 1;
-  }
 
-  // Now we move selection_start_ to beginning of selection. Selection boundary
-  // is defined as the position where we have alpha-num character on one side
-  // and non-alpha-num char on the other side.
-  for (; selection_start > 0; selection_start--) {
-    if (IsPositionAtWordSelectionBoundary(selection_start))
+  base::i18n::BreakIterator iter(text(), base::i18n::BreakIterator::BREAK_WORD);
+  bool success = iter.Init();
+  DCHECK(success);
+  if (!success)
+    return;
+
+  size_t selection_start = cursor_position;
+  for (; selection_start != 0; --selection_start) {
+    if (iter.IsStartOfWord(selection_start) ||
+        iter.IsEndOfWord(selection_start))
       break;
   }
 
-  // Now we move selection_end_ to end of selection. Selection boundary
-  // is defined as the position where we have alpha-num character on one side
-  // and non-alpha-num char on the other side.
-  for (; cursor_position < text().length(); cursor_position++) {
-    if (IsPositionAtWordSelectionBoundary(cursor_position))
+  if (selection_start == cursor_position)
+    ++cursor_position;
+
+  for (; cursor_position < text().length(); ++cursor_position) {
+    if (iter.IsEndOfWord(cursor_position) ||
+        iter.IsStartOfWord(cursor_position))
       break;
   }
 
   MoveCursorTo(selection_start, false);
   MoveCursorTo(cursor_position, true);
 }
 
 const ui::Range& RenderText::GetCompositionRange() const {
   return composition_range_;
 }
@@ skipping 126 matching lines @@
   size_t from = selection.selection_end();
   size_t to = insert_mode ? from : std::min(text_.length(), from + 1);
   return GetSubstringBounds(from, to)[0];
 }
 
 const Rect& RenderText::GetUpdatedCursorBounds() {
   UpdateCachedBoundsAndOffset();
   return cursor_bounds_;
 }
 
+size_t RenderText::GetIndexOfNextGrapheme(size_t position) {
+  // TODO(xji): Handle complex script.
+  return std::min(static_cast<long>(position + 1),

[msw, 2011/09/14 02:43:48]

Both of these static casts have lint errors:
"Use int16/int64/etc, rather than the C type long"
If it's an easy fix, please also fix RenderText::GetIndexOfPreviousGrapheme().

Or, you could take this opportunity to merge the win/linux impls that just call
GetIndexOfAdjacentGrapheme (with true/false and left/right) into these base
impls. I think you can make RenderText::GetIndexOfAdjacentGrapheme pure virtual
and just leave the implementations to RenderText[Win/Linux].

[xji, 2011/09/15 23:38:13]

Merged win/linux impls.

+                  static_cast<long>(text_.length()));
+}
+
 RenderText::RenderText()
     : text_(),
       selection_model_(),
       cursor_bounds_(),
       cursor_visible_(false),
       insert_mode_(true),
       composition_range_(ui::Range::InvalidRange()),
       style_ranges_(),
       default_style_(),
       display_rect_(),
@@ skipping 93 matching lines @@
   int start_x = font.GetStringWidth(text().substr(0, start));
   int end_x = font.GetStringWidth(text().substr(0, end));
   Rect rect(start_x, 0, end_x - start_x, font.GetHeight());
   rect.Offset(display_rect_.origin());
   rect.Offset(GetUpdatedDisplayOffset());
   // Center the rect vertically in |display_rect_|.
   rect.Offset(Point(0, (display_rect_.height() - rect.height()) / 2));
   return std::vector<Rect>(1, rect);
 }
 
+bool RenderText::IsCursorablePosition(size_t position) {
+  return true;

[msw, 2011/09/14 02:43:48]

I think this can potentially be pure virtual, and leave the impls to win/linux.

[xji, 2011/09/15 23:38:13]

Done.

+}
+
 void RenderText::ApplyCompositionAndSelectionStyles(
     StyleRanges* style_ranges) const {
   // TODO(msw): This pattern ought to be reconsidered; what about composition
   //            and selection overlaps, retain existing local style features?
   // Apply a composition style override to a copy of the style ranges.
   if (composition_range_.IsValid() && !composition_range_.is_empty()) {
     StyleRange composition_style(default_style_);
     composition_style.underline = true;
     composition_style.range.set_start(composition_range_.start());
     composition_style.range.set_end(composition_range_.end());
@@ skipping 37 matching lines @@
 
   cached_bounds_and_offset_valid_ = false;
 }
 
 void RenderText::MoveCursorTo(size_t position, bool select) {
   size_t cursor = std::min(position, text().length());
   size_t caret_pos = GetIndexOfPreviousGrapheme(cursor);
   SelectionModel::CaretPlacement placement = (caret_pos == cursor) ?
       SelectionModel::LEADING : SelectionModel::TRAILING;
   size_t selection_start = select ? GetSelectionStart() : cursor;
-  SelectionModel sel(selection_start, cursor, caret_pos, placement);
-  SetSelectionModel(sel);
-}
-
-bool RenderText::IsPositionAtWordSelectionBoundary(size_t pos) {
-  return pos == 0 || (u_isalnum(text()[pos - 1]) && !u_isalnum(text()[pos])) ||
-      (!u_isalnum(text()[pos - 1]) && u_isalnum(text()[pos]));
+  if (IsCursorablePosition(cursor)) {
+    SelectionModel sel(selection_start, cursor, caret_pos, placement);
+    SetSelectionModel(sel);
+  }
 }
 
 void RenderText::UpdateCachedBoundsAndOffset() {
   if (cached_bounds_and_offset_valid_)
     return;
   // First, set the valid flag true to calculate the current cursor bounds using
   // the stale |display_offset_|. Applying |delta_offset| at the end of this
   // function will set |cursor_bounds_| and |display_offset_| to correct values.
   cached_bounds_and_offset_valid_ = true;
   cursor_bounds_ = GetCursorBounds(selection_model_, insert_mode_);
@@ skipping 30 matching lines @@
                           selection_start,
                           SelectionModel::LEADING);
   else if (selection_start > selection_end)
     return SelectionModel(selection_start,
                           GetIndexOfPreviousGrapheme(selection_start),
                           SelectionModel::TRAILING);
   return selection_model_;
 }
 
 }  // namespace gfx