Commit 9abf5737 authored by W. James MacLean's avatar W. James MacLean Committed by Commit Bot

Don't default to allowing handles to be shown.

At present TouchSelectionController uses |show_touch_handles_| to decide
when handles are shown by touch events. It is set to 'true' in
TouchSelectionController::WillHandleTouchEventImpl(). But if this value
is defaulted to true, then PDFs may show the handles on the first
selection, even if initiated by non-touch means. This doesn't show up
in non-PDF pages, as there seems to be a mechanism in
RenderWidgetHostViewAura that prevents the TouchSelectionController from
ever being told about selection changes from non-Touch events.

Some tests were fixed, since they relied on the old default value to
avoid having to send actual touch events.

The CL changes the default to avoid handles showing up for the first
mouse selection in PDFs.

Bug: 933631
Change-Id: I1d66df070c22481141f68f6085f04878842c9f2b
Reviewed-on: https://chromium-review.googlesource.com/c/1491794
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: default avatarMohsen Izadi <mohsen@chromium.org>
Reviewed-by: default avatarAJITH KUMAR V <ajith.v@samsung.com>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636809}
parent e18d417f
......@@ -31,6 +31,7 @@
#include "ui/events/event_sink.h"
#include "ui/events/event_utils.h"
#include "ui/events/test/event_generator.h"
#include "ui/events/test/motion_event_test_utils.h"
#include "ui/touch_selection/touch_selection_controller_test_api.h"
namespace content {
......@@ -177,6 +178,9 @@ class TouchSelectionControllerClientAuraTest : public ContentBrowserTest {
new TestTouchSelectionControllerClientAura(rwhva);
rwhva->SetSelectionControllerClientForTest(
base::WrapUnique(selection_controller_client_));
// Simulate the start of a motion event sequence, since the tests assume it.
rwhva->selection_controller()->WillHandleTouchEvent(
ui::test::MockMotionEvent(ui::MotionEvent::Action::DOWN));
}
void SetUpOnMainThread() override {
......
......@@ -66,8 +66,7 @@ TouchSelectionController::TouchSelectionController(
longpress_drag_selector_(this),
selection_handle_dragged_(false),
consume_touch_sequence_(false),
show_touch_handles_(true)
{
show_touch_handles_(false) {
DCHECK(client_);
}
......
......@@ -65,6 +65,9 @@ class TouchSelectionControllerTest : public testing::Test,
void SetUp() override {
controller_.reset(new TouchSelectionController(this, DefaultConfig()));
// Simulate start of a TouchEvent sequence.
controller_->WillHandleTouchEvent(
MockMotionEvent(MotionEvent::Action::DOWN));
}
void TearDown() override { controller_.reset(); }
......@@ -121,6 +124,9 @@ class TouchSelectionControllerTest : public testing::Test,
TouchSelectionController::Config config = DefaultConfig();
config.hide_active_handle = hide;
controller_.reset(new TouchSelectionController(this, config));
// Simulate start of a TouchEvent sequence.
controller_->WillHandleTouchEvent(
MockMotionEvent(MotionEvent::Action::DOWN));
}
void SetAnimationEnabled(bool enabled) { animation_enabled_ = enabled; }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment