Simplify handling of BiDi cursor movement

ui/views/controls/textfield/native_textfield_views.cc

 // Copyright (c) 2012 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/views/controls/textfield/native_textfield_views.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 168 matching lines @@
   return ui::DragDropTypes::DRAG_COPY | ui::DragDropTypes::DRAG_MOVE;
 }
 
 int NativeTextfieldViews::OnPerformDrop(const DropTargetEvent& event) {
   DCHECK(CanDrop(event.data()));
   DCHECK(!initiating_drag_ ||
          !GetRenderText()->IsPointInSelection(event.location()));
   OnBeforeUserAction();
   skip_input_method_cancel_composition_ = true;
 
-  // TODO(msw): Remove final reference to FindCursorPosition.
-  gfx::SelectionModel drop_destination =
+  gfx::SelectionModel drop_destination_model =
       GetRenderText()->FindCursorPosition(event.location());
   string16 text;
   event.data().GetString(&text);
 
   // We'll delete the current selection for a drag and drop within this view.
   bool move = initiating_drag_ && !event.IsControlDown() &&
               event.source_operations() & ui::DragDropTypes::DRAG_MOVE;
   if (move) {
     gfx::SelectionModel selected;
     model_->GetSelectionModel(&selected);
     // Adjust the drop destination if it is on or after the current selection.
-    size_t max_of_selected_range = std::max(selected.selection_start(),
-                                            selected.selection_end());
-    size_t min_of_selected_range = std::min(selected.selection_start(),
-                                            selected.selection_end());
-    size_t selected_range_length = max_of_selected_range -
-                                   min_of_selected_range;
-    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);
+    size_t drop_destination = drop_destination_model.caret_pos();
+    drop_destination -=
+        selected.selection().Intersect(ui::Range(0, drop_destination)).length();
+    model_->DeleteSelectionAndInsertTextAt(text, drop_destination);
   } else {
-    model_->MoveCursorTo(drop_destination);
+    model_->MoveCursorTo(drop_destination_model);
     // Drop always inserts text even if the textfield is not in insert mode.
     model_->InsertText(text);
   }
   skip_input_method_cancel_composition_ = false;
   UpdateAfterChange(true, true);
   OnAfterUserAction();
   return move ? ui::DragDropTypes::DRAG_MOVE : ui::DragDropTypes::DRAG_COPY;
 }
 
 void NativeTextfieldViews::OnDragDone() {
@@ skipping 15 matching lines @@
 
 void NativeTextfieldViews::OnBlur() {
   NOTREACHED();
 }
 
 void NativeTextfieldViews::SelectRect(const gfx::Point& start,
                                       const gfx::Point& end) {
   if (GetTextInputType() == ui::TEXT_INPUT_TYPE_NONE)
     return;
 
-  gfx::SelectionModel start_pos = GetRenderText()->FindCursorPosition(start);
-  gfx::SelectionModel end_pos = GetRenderText()->FindCursorPosition(end);
+  gfx::SelectionModel selection = GetRenderText()->FindCursorPosition(end);
+  selection.set_selection_start(

[msw, 2012/02/21 21:13:39]

I agree with xji's reasoning for keeping SelectionModel::set_selection_start()
private. Change this back to your Patch Set 2 version; sorry for the bad tip.

[benrg, 2012/02/21 23:21:45]

I rewrote it to no longer use set_selection_start, but I didn't restore the more
complicated old logic (which I think  was unnecessary -- see earlier comment).
See also my response to xji.

+      GetRenderText()->FindCursorPosition(start).caret_pos());
 
   OnBeforeUserAction();
-  // 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".
-  if (start_pos.selection_start() == end_pos.selection_end())
-    model_->SelectSelectionModel(end_pos);
-  else
-    model_->SelectRange(ui::Range(start_pos.selection_start(),
-                                  end_pos.selection_end()));
-
+  model_->SelectSelectionModel(selection);
   OnCaretBoundsChanged();
   SchedulePaint();
   OnAfterUserAction();
 }
 
 gfx::NativeCursor NativeTextfieldViews::GetCursor(const MouseEvent& event) {
   bool in_selection = GetRenderText()->IsPointInSelection(event.location());
   bool drag_event = event.type() == ui::ET_MOUSE_DRAGGED;
   bool text_cursor = !initiating_drag_ && (drag_event || !in_selection);
 #if defined(USE_AURA)
@@ skipping 183 matching lines @@
 gfx::NativeView NativeTextfieldViews::GetTestingHandle() const {
   NOTREACHED();
   return NULL;
 }
 
 bool NativeTextfieldViews::IsIMEComposing() const {
   return model_->HasCompositionText();
 }
 
 void NativeTextfieldViews::GetSelectedRange(ui::Range* range) const {
-  model_->GetSelectedRange(range);
+  *range = GetRenderText()->selection();
 }
 
 void NativeTextfieldViews::SelectRange(const ui::Range& range) {
   model_->SelectRange(range);
   OnCaretBoundsChanged();
   SchedulePaint();
   textfield_->GetWidget()->NotifyAccessibilityEvent(
       textfield_, ui::AccessibilityTypes::EVENT_SELECTION_CHANGED, true);
 }
 
@@ skipping 253 matching lines @@
   model_->GetCompositionTextRange(range);
   return true;
 }
 
 bool NativeTextfieldViews::GetSelectionRange(ui::Range* range) {
   if (!ImeEditingAllowed())
     return false;
 
   gfx::SelectionModel sel;
   model_->GetSelectionModel(&sel);
-  range->set_start(sel.selection_start());
-  range->set_end(sel.selection_end());
+  *range = sel.selection();
   return true;
 }
 
 bool NativeTextfieldViews::SetSelectionRange(const ui::Range& range) {
   if (!ImeEditingAllowed() || !range.IsValid())
     return false;
 
   OnBeforeUserAction();
   SelectRange(range);
   OnAfterUserAction();
@@ skipping 364 matching lines @@
 
 #if defined(USE_AURA)
 // static
 NativeTextfieldWrapper* NativeTextfieldWrapper::CreateWrapper(
     Textfield* field) {
   return new NativeTextfieldViews(field);
 }
 #endif
 
 }  // namespace views