New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 621063 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
inactive
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Longpress on a link within a PDF is broken

Reported by jshan...@etouch.net, Jun 17 2016

Issue description

Chrome Version: 53.0.2770.0 (Official Build) 318e6f543c58eeeac93b122030041139da7e1e6a-refs/heads/master@{#400326}-32/64 bit
OS: Windows 10 (Touch device)

URL: http://www.orimi.com/pdf-test.pdf

Steps:
1. Launch Chrome and navigate to above URL.
2. Long tap/touch on the link seen on pdf page and observe

Actual: Context menu does not open for link via long tap/touch on it. 

Expected: Context menu should open for link via long tap/touch on it. 

This is a regression issue broken in M-51, below is bisect info.

Good build: 51.0.2701.0
Bad build: 51.0.2702.0

Narrow bisect:
https://chromium.googlesource.com/chromium/src/+log/5a73176ca2aabb80616c95d25f01fc3c95cb5d26..54ad52b6a815e0aaca6391f7baac2e1064d4acec?pretty=fuller&n=100

Suspecting: r385360 ?

Please help to re-assign if your change is not the cause for this issue.

Note: This is touch device specific issue, same works fine via mouse click.


 
Actual_video.mp4
454 KB View Download
Expected_video.mp4
132 KB View Download

Comment 1 by aelias@chromium.org, Jun 17 2016

Cc: thestig@chromium.org amaralp@chromium.org
Components: -UI>Browser>Options Internals>Plugins>PDF
Summary: Longpress on a link within a PDF is broken (was: Regression: Context menu does not open for link via long tap/touch on it at 'orimi.com')
My patch changed to stop selecting text in some cases on longpress.  I suppose I'll revert the behavior back to being Android-specific, but I find this regression pretty surprising, since context menus are typically a completely separate codepath.  I guess the PDF plugin might be hooking into the onSelectStart event for this purpose instead of using onContextMenu as I would've expected?  thestig@, do you have any clue where that code might live -- I'm not having any luck searching for it in ppapi/ or third_party/pdfium/.

Comment 2 by aelias@chromium.org, Jun 17 2016

Never mind, I think the real explanation is less convoluted, amaralp@ pointed out the culprit is probably the canStartSelection check originally introduced in https://codereview.chromium.org/309553007, causing longpress to be absorbed by the selection code more often and not propagated to the plugin.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3

commit a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3
Author: aelias <aelias@chromium.org>
Date: Wed Jun 22 18:27:41 2016

Make plugin elements return false from canStartSelection.

The canStartSelection property of Node is to indicate that text
selection can start there, i.e. it's some variety of text or editable.
Image and media elements, for example, are excluded.

After I switched long-press selection to consider this property in
r385360, context menu events stopped being forwarded correctly to
plugins because the events were absorbed by text selection instead.  The
root cause was this incorrectly assigned Node property.

For completeness:
- I made the long-press layout tests run on all platforms.  After
  r385360, they were no longer Android-specific, and I only just learned
  of their existence now.

BUG= 621063 

Review-Url: https://codereview.chromium.org/2078143002
Cr-Commit-Position: refs/heads/master@{#401359}

[modify] https://crrev.com/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3/third_party/WebKit/LayoutTests/NeverFixTests
[modify] https://crrev.com/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3/third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html
[modify] https://crrev.com/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp
[modify] https://crrev.com/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3/third_party/WebKit/Source/core/html/HTMLPlugInElement.h
[modify] https://crrev.com/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[add] https://crrev.com/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3/third_party/WebKit/Source/web/tests/data/long_press_object.html
[add] https://crrev.com/a1c7f43d9e5166bf0b21fc3690728e79bb6fd2a3/third_party/WebKit/Source/web/tests/data/long_press_object_fallback.html

Comment 4 by aelias@chromium.org, Jun 29 2016

Status: Fixed (was: Assigned)

Sign in to add a comment