Clean up of SelectionModel

views/controls/textfield/native_textfield_views.cc

Index: views/controls/textfield/native_textfield_views.cc
===================================================================
--- views/controls/textfield/native_textfield_views.cc	(revision 103984)
+++ views/controls/textfield/native_textfield_views.cc	(working copy)
@@ -226,15 +226,13 @@
                                             selected.selection_end());
     size_t selected_range_length = max_of_selected_range -
                                    min_of_selected_range;
-    if (max_of_selected_range <= drop_destination.selection_end())
-      drop_destination.set_selection_end(
-          drop_destination.selection_end() - selected_range_length);
-    else if (min_of_selected_range <= drop_destination.selection_end())
-      drop_destination.set_selection_end(min_of_selected_range);
-    model_->DeleteSelectionAndInsertTextAt(text,
-                                           drop_destination.selection_end());
+    size_t drop_destination_end = drop_destination.selection_end();
+    if (max_of_selected_range <= drop_destination_end)
+      drop_destination_end -= selected_range_length;
+    else if (min_of_selected_range <= drop_destination_end)
+      drop_destination_end = min_of_selected_range;
+    model_->DeleteSelectionAndInsertTextAt(text, drop_destination_end);
   } else {
-    drop_destination.set_selection_start(drop_destination.selection_end());
     model_->MoveCursorTo(drop_destination);
     // Drop always inserts text even if the textfield is not in insert mode.
     model_->InsertText(text);
@@ -278,9 +276,11 @@
   // Merge selection models of "start_pos" and "end_pos" so that
   // selection start is the value from "start_pos", while selection end,
   // caret position, and caret placement are values from "end_pos".
-  gfx::SelectionModel sel(end_pos);
-  sel.set_selection_start(start_pos.selection_start());
-  model_->SelectSelectionModel(sel);
+  if (start_pos.selection_start() == end_pos.selection_end())

[msw, 2011/10/07 20:54:20]

Is it always safe to just use the SelectRange statement? Could the end_pos
caret/placement differ from those that would be set by SelectRange?

[xji, 2011/10/07 21:42:04]

I think you are asking about line 282.
When there is non-empty selection, using SelectRange (instead of setting only
the selection_start) is the right way.

For example, logical text "abcDEFhij", visually display as "abcFEDhij", if end's
selection model is (3, 3, 2, trailing), which means cursor is at "abc|FEDhij",
and if start's logical index is 4. the selection should be (4, 3) and [D] is
selected. The selection model should be (4, 3, 3, leading) which is what
SelectRange computed, not as (4, 3, 2, trailing) which is what we get by only
set_selection_start.

+    model_->SelectSelectionModel(end_pos);
+  else
+    model_->SelectRange(ui::Range(start_pos.selection_start(),
+                                  end_pos.selection_end()));
 
   OnCaretBoundsChanged();
   SchedulePaint();
@@ -490,6 +490,16 @@
   return model_->HasCompositionText();
 }
 
+void NativeTextfieldViews::GetSelectedRange(ui::Range* range) const {
+  model_->GetSelectedRange(range);
+}
+
+void NativeTextfieldViews::SelectRange(const ui::Range& range) {
+  model_->SelectRange(range);
+  OnCaretBoundsChanged();
+  SchedulePaint();
+}
+
 void NativeTextfieldViews::GetSelectionModel(gfx::SelectionModel* sel) const {
   model_->GetSelectionModel(sel);
 }
@@ -758,7 +768,7 @@
     return false;
 
   OnBeforeUserAction();
-  SelectSelectionModel(gfx::SelectionModel(range.start(), range.end()));
+  SelectRange(range);
   OnAfterUserAction();
   return true;
 }
@@ -768,8 +778,7 @@
     return false;
 
   OnBeforeUserAction();
-  gfx::SelectionModel selection(range.start(), range.end());
-  model_->SelectSelectionModel(selection);
+  model_->SelectRange(range);
   if (model_->HasSelection()) {
     model_->DeleteSelection();
     UpdateAfterChange(true, true);