Skip to content

Commit

Permalink
Reland: Handle long press in PDF documents.
Browse files Browse the repository at this point in the history
This Cl updates the touch handlers for PDF documents to better support
long press. The long press context menu is suppressed and the wonder
under the press is of sufficient time.

BUG=chromium:490184
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

PRIOR REVIEWS:
> Review-Url: https://codereview.chromium.org/2855953003
> Revert Review-Url: https://codereview.chromium.org/2864603006

Review-Url: https://codereview.chromium.org/2861383003
Cr-Commit-Position: refs/heads/master@{#471915}
  • Loading branch information
dj2 authored and Commit bot committed May 15, 2017
1 parent ce7c55b commit c40173a
Show file tree
Hide file tree
Showing 12 changed files with 287 additions and 51 deletions.
4 changes: 4 additions & 0 deletions chrome/browser/pdf/pdf_extension_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest, GestureDetector) {
RunTestsInFile("gesture_detector_test.js", "test.pdf");
}

IN_PROC_BROWSER_TEST_F(PDFExtensionTest, TouchHandling) {
RunTestsInFile("touch_handling_test.js", "test.pdf");
}

IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Elements) {
// Although this test file does not require a PDF to be loaded, loading the
// elements without loading a PDF is difficult.
Expand Down
27 changes: 19 additions & 8 deletions chrome/browser/resources/pdf/gesture_detector.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class GestureDetector {
'touchcancel', this.onTouch_.bind(this), { passive: true });

this.pinchStartEvent_ = null;
this.lastTouchTouchesCount_ = 0;
this.lastEvent_ = null;

this.listeners_ = new Map([
Expand All @@ -46,6 +47,14 @@ class GestureDetector {
}
}

/**
* Returns true if the last touch start was a two finger touch.
* @return {boolean} True if the last touch start was a two finger touch.
*/
wasTwoFingerTouch() {
return this.lastTouchTouchesCount_ == 2;
}

/**
* Call the relevant listeners with the given |pinchEvent|.
* @private
Expand All @@ -64,14 +73,16 @@ class GestureDetector {
* @param {!TouchEvent} event Touch event on the element.
*/
onTouchStart_(event) {
if (event.touches.length == 2) {
this.pinchStartEvent_ = event;
this.lastEvent_ = event;
this.notify_({
type: 'pinchstart',
center: GestureDetector.center_(event)
});
}
this.lastTouchTouchesCount_ = event.touches.length;
if (!this.wasTwoFingerTouch())
return;

this.pinchStartEvent_ = event;
this.lastEvent_ = event;
this.notify_({
type: 'pinchstart',
center: GestureDetector.center_(event)
});
}

/**
Expand Down
13 changes: 13 additions & 0 deletions chrome/browser/resources/pdf/pdf.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ function PDFViewer(browserApi) {
document.addEventListener('keydown', this.handleKeyEvent_.bind(this));
document.addEventListener('mousemove', this.handleMouseEvent_.bind(this));
document.addEventListener('mouseout', this.handleMouseEvent_.bind(this));
document.addEventListener('contextmenu',
this.handleContextMenuEvent_.bind(this));

var tabId = this.browserApi_.getStreamInfo().tabId;
this.navigator_ = new Navigator(
Expand Down Expand Up @@ -410,6 +412,17 @@ PDFViewer.prototype = {
this.toolbarManager_.hideToolbarsForMouseOut();
},

handleContextMenuEvent_: function(e) {
// Stop Chrome from popping up the context menu on long press. We need to
// make sure the start event did not have 2 touches because we don't want
// to block two finger tap opening the context menu. We check for
// firesTouchEvents in order to not block the context menu on right click.
if (e.sourceCapabilities.firesTouchEvents &&
!this.gestureDetector_.wasTwoFingerTouch()) {
e.preventDefault();
}
},

/**
* @private
* Rotate the plugin clockwise.
Expand Down
37 changes: 37 additions & 0 deletions chrome/test/data/pdf/gesture_detector_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,43 @@ chrome.test.runTests(function() {
chrome.test.assertEq('pinchupdate', pinchListener.lastEvent.type);
chrome.test.assertTrue(pinchUpdateEvent.defaultPrevented);

chrome.test.succeed();
},

function testWasTwoFingerTouch() {
let stubElement = new StubElement();
let gestureDetector = new GestureDetector(stubElement);


chrome.test.assertFalse(gestureDetector.wasTwoFingerTouch(),
"Should not have two finger touch before first touch event.");

stubElement.sendEvent(new MockTouchEvent('touchstart', [
{clientX: 0, clientY: 0}
]));
chrome.test.assertFalse(gestureDetector.wasTwoFingerTouch(),
"Should not have a two finger touch with one touch.");

stubElement.sendEvent(new MockTouchEvent('touchstart', [
{clientX: 0, clientY: 0},
{clientX: 2, clientY: 2}
]));
chrome.test.assertTrue(gestureDetector.wasTwoFingerTouch(),
"Should have a two finger touch.");

// Make sure we keep |wasTwoFingerTouch| true after the end event.
stubElement.sendEvent(new MockTouchEvent('touchend', []));
chrome.test.assertTrue(gestureDetector.wasTwoFingerTouch(),
"Should maintain two finger touch after touchend.");

stubElement.sendEvent(new MockTouchEvent('touchstart', [
{clientX: 0, clientY: 0},
{clientX: 2, clientY: 2},
{clientX: 4, clientY: 4}
]));
chrome.test.assertFalse(gestureDetector.wasTwoFingerTouch(),
"Should not have two finger touch with 3 touches.");

chrome.test.succeed();
}
];
Expand Down
73 changes: 73 additions & 0 deletions chrome/test/data/pdf/touch_handling_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright 2017 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.

function sendTouchStart(touches) {
var id = 0;
let touchList = touches.map(function(xy) {
var touchInit = {
identifier: id++,
target: viewer.plugin_,
clientX: xy.x,
clientY: xy.y,
};

return new window.Touch(touchInit);
});

viewer.plugin_.dispatchEvent(new TouchEvent('touchstart', {
touches: touchList,
targetTouches: touchList,
changedtouches: touchList
}));
}

function createContextMenuEvent() {
return new MouseEvent('contextmenu', {
cancelable: true,
sourceCapabilities: new InputDeviceCapabilities({firesTouchEvents: true})
});
}

var tests = [
// Test suppression of the context menu on single touch.
function testContextMenuSingleTouch() {
sendTouchStart([{x: 10, y: 10}]);

let event = createContextMenuEvent();
// Dispatch event will be false if the event is cancellable and one of the
// handlers called preventDefault.
chrome.test.assertFalse(document.dispatchEvent(event),
"Should have called preventDefault() for single touch.");
chrome.test.succeed();
},

// Test allowing of context menu on double touch.
function testContextMenuDoubleTouch() {
sendTouchStart([{x: 10, y: 10}, {x: 15, y: 15}]);

let event = createContextMenuEvent();
chrome.test.assertTrue(document.dispatchEvent(event),
"Should not have called preventDefault() for double touch.");
chrome.test.succeed();
},

// Test long press selects word.
function testLongPressSelectsText() {
var client = new PDFScriptingAPI(window, window);

sendTouchStart([{x: 336, y: 163}]);
window.setTimeout(function() {
client.getSelectedText(chrome.test.callbackPass(function(selectedText) {
chrome.test.assertEq('some', selectedText);
}));
chrome.test.succeed();
// 10k is the value for the action_timeout_ms_ in Chrome test_timeouts.cc
}, 10000);
}
];

var scriptingAPI = new PDFScriptingAPI(window, window);
scriptingAPI.setLoadCallback(function() {
chrome.test.runTests(tests);
});
77 changes: 52 additions & 25 deletions pdf/out_of_process_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,11 @@ bool IsPrintPreviewUrl(base::StringPiece url) {
return url.starts_with(kChromePrint);
}

void ScaleFloatPoint(float scale, pp::FloatPoint* point) {
point->set_x(point->x() * scale);
point->set_y(point->y() * scale);
}

void ScalePoint(float scale, pp::Point* point) {
point->set_x(static_cast<int>(point->x() * scale));
point->set_y(static_cast<int>(point->y() * scale));
Expand Down Expand Up @@ -575,15 +580,16 @@ void OutOfProcessInstance::HandleMessage(const pp::Var& message) {
}

bool OutOfProcessInstance::HandleInputEvent(const pp::InputEvent& event) {
// To simplify things, convert the event into device coordinates if it is
// a mouse event.
// To simplify things, convert the event into device coordinates.
pp::InputEvent event_device_res(event);
{
pp::MouseInputEvent mouse_event(event);
if (!mouse_event.is_null()) {
pp::Point point = mouse_event.GetPosition();
pp::Point movement = mouse_event.GetMovement();
ScalePoint(device_scale_, &point);
point.set_x(point.x() - available_area_.x());

ScalePoint(device_scale_, &movement);
mouse_event =
pp::MouseInputEvent(this, event.GetType(), event.GetTimeStamp(),
Expand All @@ -592,28 +598,39 @@ bool OutOfProcessInstance::HandleInputEvent(const pp::InputEvent& event) {
event_device_res = mouse_event;
}
}

pp::InputEvent offset_event(event_device_res);
switch (offset_event.GetType()) {
case PP_INPUTEVENT_TYPE_MOUSEDOWN:
case PP_INPUTEVENT_TYPE_MOUSEUP:
case PP_INPUTEVENT_TYPE_MOUSEMOVE:
case PP_INPUTEVENT_TYPE_MOUSEENTER:
case PP_INPUTEVENT_TYPE_MOUSELEAVE: {
pp::MouseInputEvent mouse_event(event_device_res);
pp::MouseInputEvent mouse_event_dip(event);
pp::Point point = mouse_event.GetPosition();
point.set_x(point.x() - available_area_.x());
offset_event = pp::MouseInputEvent(
this, event.GetType(), event.GetTimeStamp(), event.GetModifiers(),
mouse_event.GetButton(), point, mouse_event.GetClickCount(),
mouse_event.GetMovement());
break;
{
pp::TouchInputEvent touch_event(event);
if (!touch_event.is_null()) {
pp::TouchInputEvent new_touch_event = pp::TouchInputEvent(
this, touch_event.GetType(), touch_event.GetTimeStamp(),
touch_event.GetModifiers());

for (uint32_t i = 0;
i < touch_event.GetTouchCount(PP_TOUCHLIST_TYPE_TARGETTOUCHES);
i++) {
pp::TouchPoint touch_point =
touch_event.GetTouchByIndex(PP_TOUCHLIST_TYPE_TARGETTOUCHES, i);

pp::FloatPoint point = touch_point.position();

// Account for the scroll position. Touch events are in DOM coordinates
// where mouse events appear to be in screen coordinates.
point.set_x(scroll_offset_.x() + point.x());
point.set_y(scroll_offset_.y() + point.y());
ScaleFloatPoint(device_scale_, &point);

point.set_x(point.x() - available_area_.x());

new_touch_event.AddTouchPoint(
PP_TOUCHLIST_TYPE_TARGETTOUCHES,
{touch_point.id(), point, touch_point.radii(),
touch_point.rotation_angle(), touch_point.pressure()});
}
event_device_res = new_touch_event;
}
default:
break;
}
if (engine_->HandleEvent(offset_event))

if (engine_->HandleEvent(event_device_res))
return true;

// Middle click is used for scrolling and is handled by the container page.
Expand Down Expand Up @@ -658,13 +675,13 @@ void OutOfProcessInstance::DidChangeView(const pp::View& view) {
}

if (!stop_scrolling_) {
pp::Point scroll_offset(view.GetScrollOffset());
scroll_offset_ = view.GetScrollOffset();
// Because view messages come from the DOM, the coordinates of the viewport
// are 0-based (i.e. they do not correspond to the viewport's coordinates in
// JS), so we need to subtract the toolbar height to convert them into
// viewport coordinates.
pp::FloatPoint scroll_offset_float(scroll_offset.x(),
scroll_offset.y() - top_toolbar_height_);
pp::FloatPoint scroll_offset_float(
scroll_offset_.x(), scroll_offset_.y() - top_toolbar_height_);
scroll_offset_float = BoundScrollOffsetToDocument(scroll_offset_float);
engine_->ScrolledToXPosition(scroll_offset_float.x() * device_scale_);
engine_->ScrolledToYPosition(scroll_offset_float.y() * device_scale_);
Expand Down Expand Up @@ -961,6 +978,10 @@ void OutOfProcessInstance::DidOpenPreview(int32_t result) {
}
}

void OutOfProcessInstance::OnClientTouchTimerFired(int32_t id) {
engine_->OnTouchTimerCallback(id);
}

void OutOfProcessInstance::OnClientTimerFired(int32_t id) {
engine_->OnCallback(id);
}
Expand Down Expand Up @@ -1249,6 +1270,12 @@ pp::URLLoader OutOfProcessInstance::CreateURLLoader() {
return CreateURLLoaderInternal();
}

void OutOfProcessInstance::ScheduleTouchTimerCallback(int id, int delay_in_ms) {
pp::CompletionCallback callback = callback_factory_.NewCallback(
&OutOfProcessInstance::OnClientTouchTimerFired);
pp::Module::Get()->core()->CallOnMainThread(delay_in_ms, callback, id);
}

void OutOfProcessInstance::ScheduleCallback(int id, int delay_in_ms) {
pp::CompletionCallback callback =
callback_factory_.NewCallback(&OutOfProcessInstance::OnClientTimerFired);
Expand Down
4 changes: 4 additions & 0 deletions pdf/out_of_process_instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class OutOfProcessInstance : public pp::Instance,

// Called when the timer is fired.
void OnClientTimerFired(int32_t id);
void OnClientTouchTimerFired(int32_t id);

// Called to print without re-entrancy issues.
void OnPrint(int32_t);
Expand Down Expand Up @@ -120,6 +121,7 @@ class OutOfProcessInstance : public pp::Instance,
std::string ShowFileSelectionDialog() override;
pp::URLLoader CreateURLLoader() override;
void ScheduleCallback(int id, int delay_in_ms) override;
void ScheduleTouchTimerCallback(int id, int delay_in_ms) override;
void SearchString(const base::char16* string,
const base::char16* term,
bool case_sensitive,
Expand Down Expand Up @@ -228,6 +230,8 @@ class OutOfProcessInstance : public pp::Instance,
// Size of entire document in pixels (i.e. if each page is 800 pixels high and
// there are 10 pages, the height will be 8000).
pp::Size document_size_;
// The scroll offset in CSS pixels.
pp::Point scroll_offset_;

// Enumeration of pinch states.
// This should match PinchPhase enum in
Expand Down
9 changes: 7 additions & 2 deletions pdf/pdf_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,12 @@ class PDFEngine {
// Creates and returns new URL loader for partial document requests.
virtual pp::URLLoader CreateURLLoader() = 0;

// Calls the client's OnCallback() function in delay_in_ms with the given
// id.
// Calls the client's OnCallback() function in |delay_in_ms| with the given
// |id|.
virtual void ScheduleCallback(int id, int delay_in_ms) = 0;
// Calls the client's OnTouchTimerCallback() function in |delay_in_ms| with
// the given |id|.
virtual void ScheduleTouchTimerCallback(int id, int delay_in_ms) = 0;

// Searches the given string for "term" and returns the results. Unicode-
// aware.
Expand Down Expand Up @@ -251,6 +254,8 @@ class PDFEngine {
virtual void SetGrayscale(bool grayscale) = 0;
// Callback for timer that's set with ScheduleCallback().
virtual void OnCallback(int id) = 0;
// Callback for timer that's set with ScheduleTouchTimerCallback().
virtual void OnTouchTimerCallback(int id) = 0;
// Get the number of characters on a given page.
virtual int GetCharCount(int page_index) = 0;
// Get the bounds in page pixels of a character on a given page.
Expand Down
Loading

0 comments on commit c40173a

Please sign in to comment.