Skip to content

Commit

Permalink
Use base and extent in TouchSelectionControllerImpl drag updates.
Browse files Browse the repository at this point in the history 
Rewrite some views touch selection methods in preparation for
implementing an "expand by word, shrink by character" feature (part of
go/cros-touch-text-editing, for visualisation of feature see
https://drive.google.com/file/d/1LdcaCgzLQCrSaqXF9gxgTPOj_l1Eetup/view?usp=sharing):

1. In TouchSelectionControllerImpl, replace SetDraggingHandle and
SelectionHandleDragged by OnDragBegin, OnDragUpdate and OnDragEnd,
based on the corresponding methods in TouchSelectionController.

2. In TouchEditable, replace MoveCaretTo and SelectRect by MoveCaret,
MoveRangeSelectionExtent and SelectBetweenCoordinates, based on the
TouchSelectionControllerClient interface.

This CL doesn't change any visible behaviour. The new "expand by word,
shrink by character" behaviour will be implemented in later CLs.

Bug: b:244116654, b:264824692
Change-Id: I0b3e3549b277297611d3f0765e0751579898d94b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4172677
Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Michelle Chen <michellegc@google.com>
Cr-Commit-Position: refs/heads/main@{#1096516}
  • Loading branch information
Michelle authored and Chromium LUCI CQ committed Jan 25, 2023
1 parent 4c6ab01 commit 9494b2d
Show file tree
Hide file tree
Showing 7 changed files with 177 additions and 52 deletions.
41 changes: 34 additions & 7 deletions ui/base/pointer/touch_editing_controller.h
View file
Expand Up @@ -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
Expand Down
37 changes: 29 additions & 8 deletions ui/views/controls/textfield/textfield.cc
View file
Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions ui/views/controls/textfield/textfield.h
View file
Expand Up @@ -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;
Expand Down
58 changes: 58 additions & 0 deletions ui/views/controls/textfield/textfield_unittest.cc
View file
Expand Up @@ -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");
Expand Down
65 changes: 38 additions & 27 deletions ui/views/touchui/touch_selection_controller_impl.cc
View file
Expand Up @@ -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).
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 4 additions & 2 deletions ui/views/touchui/touch_selection_controller_impl.h
View file
Expand Up @@ -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.
Expand Down
16 changes: 10 additions & 6 deletions ui/views/touchui/touch_selection_controller_impl_unittest.cc
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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_;
Expand Down

0 comments on commit 9494b2d

Please sign in to comment.