diff --git a/ui/base/pointer/touch_editing_controller.h b/ui/base/pointer/touch_editing_controller.h index 8e3d761a7b809..e29ff1996feac 100644 --- a/ui/base/pointer/touch_editing_controller.h +++ b/ui/base/pointer/touch_editing_controller.h @@ -33,16 +33,43 @@ class COMPONENT_EXPORT(UI_BASE) TouchEditable kLastTouchEditableCommandId = kSelectWord, }; - // TODO(mohsen): Consider switching from local coordinates to screen + // TODO(b/266345972): Consider switching from local coordinates to screen // coordinates in this interface and see if it will simplify things. - // Select everything between start and end (points are in view's local - // coordinate system). |start| is the logical start and |end| is the logical - // end of selection. Visually, |start| may lie after |end|. - virtual void SelectRect(const gfx::Point& start, const gfx::Point& end) = 0; + // Moves the caret to |position|. |position| is in local coordinates. + virtual void MoveCaret(const gfx::Point& position) = 0; - // Move the caret to |point|. |point| is in local coordinates. - virtual void MoveCaretTo(const gfx::Point& point) = 0; + // Moves the logical end of the selection according to |extent| while keeping + // the logical start of the selection fixed. Here, |extent| corresponds to the + // position (in local coordinates) of the touch handle being dragged to update + // the selection range. + // + // Note that the resultant end of the selection depends on the behaviour of + // the TouchEditable, e.g. for "expand by word, shrink by character", the + // selection end can move to the character or word boundary nearest to + // |extent| depending on the previous extent position: + // ____________________________________ + // | textf|ield wit|h selected text | + // ------------------------------------ + // ^extent + // ^start ^end + // + // ____________________________________ + // | textf|ield with selec|ted| text | + // ------------------------------------ + // ^extent + // ^start ^end + // + virtual void MoveRangeSelectionExtent(const gfx::Point& extent) = 0; + + // Sets the logical start and end of the selection according to |base| and + // |extent|. |base| corresponds to the position of the fixed touch handle and + // determines the logical start of the selection. |extent| corresponds to the + // position of the currently dragging handle and determines the logical end of + // the selection, which may be visually before, on, or after the logical start + // of the selection. Both |base| and |start| are in local coordinates. + virtual void SelectBetweenCoordinates(const gfx::Point& base, + const gfx::Point& extent) = 0; // Gets the end points of the current selection. The end points |anchor| and // |focus| must be the cursor rect for the logical start and logical end of diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc index 119f89e3478f3..479947174ddce 100644 --- a/ui/views/controls/textfield/textfield.cc +++ b/ui/views/controls/textfield/textfield.cc @@ -1184,23 +1184,44 @@ bool Textfield::HasTextBeingDragged() const { //////////////////////////////////////////////////////////////////////////////// // Textfield, ui::TouchEditable overrides: -void Textfield::SelectRect(const gfx::Point& start, const gfx::Point& end) { - if (GetTextInputType() == ui::TEXT_INPUT_TYPE_NONE) +void Textfield::MoveCaret(const gfx::Point& position) { + SelectBetweenCoordinates(position, position); +} + +void Textfield::MoveRangeSelectionExtent(const gfx::Point& extent) { + if (GetTextInputType() == ui::TEXT_INPUT_TYPE_NONE) { return; + } - gfx::SelectionModel start_caret = GetRenderText()->FindCursorPosition(start); - gfx::SelectionModel end_caret = GetRenderText()->FindCursorPosition(end); + gfx::SelectionModel base_caret = + GetRenderText()->GetSelectionModelForSelectionStart(); + gfx::SelectionModel extent_caret = + GetRenderText()->FindCursorPosition(extent); gfx::SelectionModel selection( - gfx::Range(start_caret.caret_pos(), end_caret.caret_pos()), - end_caret.caret_affinity()); + gfx::Range(base_caret.caret_pos(), extent_caret.caret_pos()), + extent_caret.caret_affinity()); OnBeforeUserAction(); SelectSelectionModel(selection); OnAfterUserAction(); } -void Textfield::MoveCaretTo(const gfx::Point& point) { - SelectRect(point, point); +void Textfield::SelectBetweenCoordinates(const gfx::Point& base, + const gfx::Point& extent) { + if (GetTextInputType() == ui::TEXT_INPUT_TYPE_NONE) { + return; + } + + gfx::SelectionModel base_caret = GetRenderText()->FindCursorPosition(base); + gfx::SelectionModel extent_caret = + GetRenderText()->FindCursorPosition(extent); + gfx::SelectionModel selection( + gfx::Range(base_caret.caret_pos(), extent_caret.caret_pos()), + extent_caret.caret_affinity()); + + OnBeforeUserAction(); + SelectSelectionModel(selection); + OnAfterUserAction(); } void Textfield::GetSelectionEndPoints(gfx::SelectionBound* anchor, diff --git a/ui/views/controls/textfield/textfield.h b/ui/views/controls/textfield/textfield.h index 5782d993657dc..043b0b40056f9 100644 --- a/ui/views/controls/textfield/textfield.h +++ b/ui/views/controls/textfield/textfield.h @@ -387,8 +387,10 @@ class VIEWS_EXPORT Textfield : public View, bool HasTextBeingDragged() const override; // ui::TouchEditable overrides: - void SelectRect(const gfx::Point& start, const gfx::Point& end) override; - void MoveCaretTo(const gfx::Point& point) override; + void MoveCaret(const gfx::Point& position) override; + void MoveRangeSelectionExtent(const gfx::Point& extent) override; + void SelectBetweenCoordinates(const gfx::Point& base, + const gfx::Point& extent) override; void GetSelectionEndPoints(gfx::SelectionBound* anchor, gfx::SelectionBound* focus) override; gfx::Rect GetBounds() override; diff --git a/ui/views/controls/textfield/textfield_unittest.cc b/ui/views/controls/textfield/textfield_unittest.cc index 27ade25954d51..be7755c48f615 100644 --- a/ui/views/controls/textfield/textfield_unittest.cc +++ b/ui/views/controls/textfield/textfield_unittest.cc @@ -3802,6 +3802,64 @@ TEST_F(TextfieldTouchSelectionTest, MAYBE_TapOnSelection) { EXPECT_EQ(tap_range, range); } +TEST_F(TextfieldTest, MoveCaret) { + InitTextfield(); + textfield_->SetText(u"hello world"); + const int cursor_y = GetCursorYForTesting(); + gfx::Range range; + + textfield_->MoveCaret(gfx::Point(GetCursorPositionX(3), cursor_y)); + textfield_->GetEditableSelectionRange(&range); + EXPECT_EQ(range, gfx::Range(3)); + + textfield_->MoveCaret(gfx::Point(GetCursorPositionX(0), cursor_y)); + textfield_->GetEditableSelectionRange(&range); + EXPECT_EQ(range, gfx::Range(0)); + + textfield_->MoveCaret(gfx::Point(GetCursorPositionX(11), cursor_y)); + textfield_->GetEditableSelectionRange(&range); + EXPECT_EQ(range, gfx::Range(11)); +} + +TEST_F(TextfieldTest, MoveRangeSelectionExtent) { + InitTextfield(); + textfield_->SetText(u"hello world"); + const int cursor_y = GetCursorYForTesting(); + gfx::Range range; + + textfield_->SelectBetweenCoordinates( + gfx::Point(GetCursorPositionX(2), cursor_y), + gfx::Point(GetCursorPositionX(3), cursor_y)); + textfield_->MoveRangeSelectionExtent( + gfx::Point(GetCursorPositionX(5), cursor_y)); + textfield_->GetEditableSelectionRange(&range); + EXPECT_EQ(range, gfx::Range(2, 5)); + + textfield_->MoveRangeSelectionExtent( + gfx::Point(GetCursorPositionX(0), cursor_y)); + textfield_->GetEditableSelectionRange(&range); + EXPECT_EQ(range, gfx::Range(2, 0)); +} + +TEST_F(TextfieldTest, SelectBetweenCoordinates) { + InitTextfield(); + textfield_->SetText(u"hello world"); + const int cursor_y = GetCursorYForTesting(); + gfx::Range range; + + textfield_->SelectBetweenCoordinates( + gfx::Point(GetCursorPositionX(1), cursor_y), + gfx::Point(GetCursorPositionX(2), cursor_y)); + textfield_->GetEditableSelectionRange(&range); + EXPECT_EQ(range, gfx::Range(1, 2)); + + textfield_->SelectBetweenCoordinates( + gfx::Point(GetCursorPositionX(0), cursor_y), + gfx::Point(GetCursorPositionX(11), cursor_y)); + textfield_->GetEditableSelectionRange(&range); + EXPECT_EQ(range, gfx::Range(0, 11)); +} + TEST_F(TextfieldTest, AccessiblePasswordTest) { InitTextfield(); textfield_->SetText(u"password"); diff --git a/ui/views/touchui/touch_selection_controller_impl.cc b/ui/views/touchui/touch_selection_controller_impl.cc index 60859c7f4e17e..7281f69a3740f 100644 --- a/ui/views/touchui/touch_selection_controller_impl.cc +++ b/ui/views/touchui/touch_selection_controller_impl.cc @@ -249,7 +249,7 @@ class TouchSelectionControllerImpl::EditingHandleView : public View { switch (event->type()) { case ui::ET_GESTURE_SCROLL_BEGIN: { widget_->SetCapture(this); - controller_->SetDraggingHandle(this); + controller_->OnDragBegin(this); // Distance from the point which is |kSelectionHandleVerticalDragOffset| // pixels above the bottom of the selection bound edge to the event // location (aka the touch-drag point). @@ -259,13 +259,13 @@ class TouchSelectionControllerImpl::EditingHandleView : public View { break; } case ui::ET_GESTURE_SCROLL_UPDATE: { - controller_->SelectionHandleDragged(event->location() + drag_offset_); + controller_->OnDragUpdate(event->location() + drag_offset_); break; } case ui::ET_GESTURE_SCROLL_END: case ui::ET_SCROLL_FLING_START: { widget_->ReleaseCapture(); - controller_->SetDraggingHandle(nullptr); + controller_->OnDragEnd(); break; } default: @@ -360,8 +360,7 @@ class TouchSelectionControllerImpl::EditingHandleView : public View { bool is_cursor_handle_; // Offset applied to the scroll events location when calling - // TouchSelectionControllerImpl::SelectionHandleDragged while dragging the - // handle. + // TouchSelectionControllerImpl::OnDragUpdate while dragging the handle. gfx::Vector2d drag_offset_; // If set to true, the handle will not draw anything, hence providing an empty @@ -508,40 +507,52 @@ void TouchSelectionControllerImpl::ShowQuickMenuImmediatelyForTesting() { } } -void TouchSelectionControllerImpl::SetDraggingHandle( - EditingHandleView* handle) { +void TouchSelectionControllerImpl::OnDragBegin(EditingHandleView* handle) { dragging_handle_ = handle; - if (dragging_handle_) - HideQuickMenu(); - else - StartQuickMenuTimer(); + HideQuickMenu(); + if (dragging_handle_ == cursor_handle_) { + return; + } + + DCHECK(dragging_handle_ == selection_handle_1_ || + dragging_handle_ == selection_handle_2_); + + // Find selection end points in client_view's coordinate system. + gfx::Point base = selection_bound_1_.edge_start_rounded(); + base.Offset(0, selection_bound_1_.GetHeight() / 2); + client_view_->ConvertPointFromScreen(&base); + + gfx::Point extent = selection_bound_2_.edge_start_rounded(); + extent.Offset(0, selection_bound_2_.GetHeight() / 2); + client_view_->ConvertPointFromScreen(&extent); + + if (dragging_handle_ == selection_handle_1_) { + std::swap(base, extent); + } + + // When moving the handle we want to move only the extent point. Before + // doing so we must make sure that the base point is set correctly. + client_view_->SelectBetweenCoordinates(base, extent); } -void TouchSelectionControllerImpl::SelectionHandleDragged( - const gfx::Point& drag_pos) { +void TouchSelectionControllerImpl::OnDragUpdate(const gfx::Point& drag_pos) { DCHECK(dragging_handle_); gfx::Point drag_pos_in_client = drag_pos; ConvertPointToClientView(dragging_handle_, &drag_pos_in_client); if (dragging_handle_ == cursor_handle_) { - client_view_->MoveCaretTo(drag_pos_in_client); + client_view_->MoveCaret(drag_pos_in_client); return; } - // Find the stationary selection handle. - gfx::SelectionBound anchor_bound = selection_handle_1_ == dragging_handle_ - ? selection_bound_2_ - : selection_bound_1_; + DCHECK(dragging_handle_ == selection_handle_1_ || + dragging_handle_ == selection_handle_2_); + client_view_->MoveRangeSelectionExtent(drag_pos_in_client); +} - // Find selection end points in client_view's coordinate system. - gfx::Point p2 = anchor_bound.edge_start_rounded(); - p2.Offset(0, anchor_bound.GetHeight() / 2); - client_view_->ConvertPointFromScreen(&p2); - - // Instruct client_view to select the region between p1 and p2. The position - // of |fixed_handle| is the start and that of |dragging_handle| is the end - // of selection. - client_view_->SelectRect(p2, drag_pos_in_client); +void TouchSelectionControllerImpl::OnDragEnd() { + dragging_handle_ = nullptr; + StartQuickMenuTimer(); } void TouchSelectionControllerImpl::ConvertPointToClientView( diff --git a/ui/views/touchui/touch_selection_controller_impl.h b/ui/views/touchui/touch_selection_controller_impl.h index 000bc4e1b1cb3..98377941d2dfd 100644 --- a/ui/views/touchui/touch_selection_controller_impl.h +++ b/ui/views/touchui/touch_selection_controller_impl.h @@ -47,13 +47,15 @@ class VIEWS_EXPORT TouchSelectionControllerImpl private: friend class TouchSelectionControllerImplTest; - void SetDraggingHandle(EditingHandleView* handle); + void OnDragBegin(EditingHandleView* handle); // Callback to inform the client view that the selection handle has been // dragged, hence selection may need to be updated. |drag_pos| is the new // position for the edge of the selection corresponding to |dragging_handle_|, // specified in handle's coordinates - void SelectionHandleDragged(const gfx::Point& drag_pos); + void OnDragUpdate(const gfx::Point& drag_pos); + + void OnDragEnd(); // Convenience method to convert a point from a selection handle's coordinate // system to that of the client view. diff --git a/ui/views/touchui/touch_selection_controller_impl_unittest.cc b/ui/views/touchui/touch_selection_controller_impl_unittest.cc index 7774d75c50e2c..77d3b787022ec 100644 --- a/ui/views/touchui/touch_selection_controller_impl_unittest.cc +++ b/ui/views/touchui/touch_selection_controller_impl_unittest.cc @@ -408,9 +408,9 @@ TEST_F(TouchSelectionControllerImplTest, SelectionInBidiTextfieldTest) { VerifyHandlePositions(false, true, FROM_HERE); } -// Tests if the SelectRect callback is called appropriately when selection -// handles are moved. -TEST_F(TouchSelectionControllerImplTest, SelectRectCallbackTest) { +// Tests if the selection update callbacks are called appropriately when +// selection handles are moved. +TEST_F(TouchSelectionControllerImplTest, SelectionUpdateCallbackTest) { CreateTextfield(); textfield_->SetText(u"textfield with selected text"); // Tap the textfield to invoke touch selection. @@ -452,7 +452,7 @@ TEST_F(TouchSelectionControllerImplTest, SelectRectCallbackTest) { VerifyHandlePositions(false, true, FROM_HERE); } -TEST_F(TouchSelectionControllerImplTest, SelectRectInBidiCallbackTest) { +TEST_F(TouchSelectionControllerImplTest, SelectionUpdateInBidiCallbackTest) { CreateTextfield(); textfield_->SetText( u"abc\x05e1\x05e2\x05e3" @@ -668,10 +668,14 @@ class TestTouchEditable : public ui::TouchEditable { private: // Overridden from ui::TouchEditable. - void SelectRect(const gfx::Point& start, const gfx::Point& end) override { + void MoveCaret(const gfx::Point& position) override { NOTREACHED(); } + void MoveRangeSelectionExtent(const gfx::Point& extent) override { + NOTREACHED(); + } + void SelectBetweenCoordinates(const gfx::Point& base, + const gfx::Point& extent) override { NOTREACHED(); } - void MoveCaretTo(const gfx::Point& point) override { NOTREACHED(); } void GetSelectionEndPoints(gfx::SelectionBound* anchor, gfx::SelectionBound* focus) override { *anchor = *focus = cursor_bound_;