Skip to content
This repository was archived by the owner on Apr 3, 2020. It is now read-only.

Commit 5e1e7b4

Browse files
author
Jared Duke
committed
Fix issue with longpress drag selection motion
If the user has already dragged outside the initial selected word region by the time the browser gets a selection update, or if the selected word isn't coincident with the longpress location, the subsequently dragged selection endpoint can be wrong. For such cases, rather than using the drag direction for drag anchor picking, just use the distance between the current touch point and the selection endpoints. BUG=466749 Review URL: https://codereview.chromium.org/1271823002 Cr-Commit-Position: refs/heads/master@{#341791} (cherry picked from commit a188bd6) Review URL: https://codereview.chromium.org/1259593010 . Cr-Commit-Position: refs/branch-heads/2454@{#228} Cr-Branched-From: 12bfc33-refs/heads/master@{#338390}
1 parent 30b1422 commit 5e1e7b4

File tree

2 files changed

+59
-12
lines changed

2 files changed

+59
-12
lines changed

ui/touch_selection/longpress_drag_selector.cc

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88
#include "ui/events/gesture_detection/motion_event.h"
99

1010
namespace ui {
11+
namespace {
12+
13+
gfx::Vector2dF SafeNormalize(const gfx::Vector2dF& v) {
14+
return v.IsZero() ? v : ScaleVector2d(v, 1.f / v.Length());
15+
}
16+
17+
} // namespace
1118

1219
LongPressDragSelector::LongPressDragSelector(
1320
LongPressDragSelectorClient* client)
@@ -70,21 +77,27 @@ bool LongPressDragSelector::WillHandleTouchEvent(const MotionEvent& event) {
7077
// If initial motion is up/down, extend the start/end selection bound.
7178
extend_selection_start = delta.y() < 0;
7279
} else {
73-
// Otherwise extend the selection bound toward which we're moving.
80+
// Otherwise extend the selection bound toward which we're moving, or
81+
// the closest bound if motion is already away from both bounds.
7482
// Note that, for mixed RTL text, or for multiline selections triggered
7583
// by longpress, this may not pick the most suitable drag target
76-
gfx::Vector2dF start_delta = selection_start - position;
84+
gfx::Vector2dF start_delta = selection_start - longpress_drag_start_anchor_;
85+
gfx::Vector2dF end_delta = selection_end - longpress_drag_start_anchor_;
7786

7887
// The vectors must be normalized to make dot product comparison meaningful.
79-
if (!start_delta.IsZero())
80-
start_delta.Scale(1.f / start_delta.Length());
81-
gfx::Vector2dF end_delta = selection_end - position;
82-
if (!end_delta.IsZero())
83-
end_delta.Scale(1.f / start_delta.Length());
84-
85-
// The larger the dot product the more similar the direction.
86-
extend_selection_start =
87-
gfx::DotProduct(start_delta, delta) > gfx::DotProduct(end_delta, delta);
88+
gfx::Vector2dF normalized_start_delta = SafeNormalize(start_delta);
89+
gfx::Vector2dF normalized_end_delta = SafeNormalize(end_delta);
90+
double start_dot_product = gfx::DotProduct(normalized_start_delta, delta);
91+
double end_dot_product = gfx::DotProduct(normalized_end_delta, delta);
92+
93+
if (start_dot_product >= 0 || end_dot_product >= 0) {
94+
// The greater the dot product the more similar the direction.
95+
extend_selection_start = start_dot_product > end_dot_product;
96+
} else {
97+
// If we're already moving away from both endpoints, pick the closest.
98+
extend_selection_start =
99+
start_delta.LengthSquared() < end_delta.LengthSquared();
100+
}
88101
}
89102

90103
gfx::PointF extent = extend_selection_start ? selection_start : selection_end;

ui/touch_selection/longpress_drag_selector_unittest.cc

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ TEST_F(LongPressDragSelectorTest, BasicReverseDrag) {
139139
EXPECT_FALSE(IsDragging());
140140

141141
// Initiate drag motion.
142-
EXPECT_TRUE(selector.WillHandleTouchEvent(event.MovePoint(0, 0, 0)));
142+
EXPECT_TRUE(selector.WillHandleTouchEvent(event.MovePoint(0, 5, 0)));
143143
EXPECT_FALSE(IsDragging());
144144

145145
// As the initial motion is leftward, toward the selection start, the
@@ -330,4 +330,38 @@ TEST_F(LongPressDragSelectorTest, SelectionDeactivated) {
330330
EXPECT_FALSE(selector.WillHandleTouchEvent(event.ReleasePoint()));
331331
}
332332

333+
TEST_F(LongPressDragSelectorTest, DragFast) {
334+
LongPressDragSelector selector(this);
335+
MockMotionEvent event;
336+
337+
// Start a touch sequence.
338+
EXPECT_FALSE(selector.WillHandleTouchEvent(event.PressPoint(0, 0)));
339+
EXPECT_FALSE(GetAndResetActiveStateChanged());
340+
341+
// Activate a longpress-triggered selection.
342+
gfx::PointF selection_start(0, 10);
343+
gfx::PointF selection_end(10, 10);
344+
selector.OnLongPressEvent(event.GetEventTime(), gfx::PointF());
345+
EXPECT_TRUE(GetAndResetActiveStateChanged());
346+
SetSelection(selection_start, selection_end);
347+
selector.OnSelectionActivated();
348+
EXPECT_FALSE(IsDragging());
349+
350+
// Initiate drag motion.
351+
EXPECT_TRUE(selector.WillHandleTouchEvent(event.MovePoint(0, 15, 5)));
352+
EXPECT_FALSE(IsDragging());
353+
354+
// As the initial motion exceeds both endpoints, the closer bound should
355+
// be used for dragging, in this case the selection end.
356+
EXPECT_TRUE(selector.WillHandleTouchEvent(
357+
event.MovePoint(0, 15.f + kSlop * 2.f, 5.f + kSlop)));
358+
EXPECT_TRUE(IsDragging());
359+
EXPECT_EQ(selection_end, DragPosition());
360+
361+
// Release the touch sequence, ending the drag.
362+
EXPECT_FALSE(selector.WillHandleTouchEvent(event.ReleasePoint()));
363+
EXPECT_FALSE(IsDragging());
364+
EXPECT_TRUE(GetAndResetActiveStateChanged());
365+
}
366+
333367
} // namespace ui

0 commit comments

Comments
 (0)