New issue
Advanced search Search tips

Issue 807236 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Tooltip doesn't appear for zoom in (+), Zoom out (-) and Fit to page options on for any PDF page.

Reported by dchau...@etouch.net, Jan 30 2018

Issue description

Chrome Version: 65.0.3325.31 (Official Build) Revision e78f314d5510b3d61fc4960671e23358931fec96-refs/branch-heads/3325@{#160} (32/64-bit)  
OS: Windows(7,8,8.1,10),Mac(10.12.6, 10.13.1, 10.13.3), Linux(14.04 LTS).

URL: https://collegereadiness.collegeboard.org/pdf/sat-practice-test-1.pdf

What steps will reproduce the problem?
1. Launch Chrome, go to above URL or open any PDF page.
2. Now hover the mouse pointer on zoom in (+), Zoom out (-) and Fit to page options and observe.

Tooltip doesn't appear for zoom in (+), Zoom out (-) and Fit to page options.
Tooltip should be seen for zoom in (+), Zoom out (-) and Fit to page options.

This is a regression issue, broken in M-65 series, will soon update other info.
 

Comment 1 by dchau...@etouch.net, Jan 30 2018

Labels: hasbisect-per-revision RegressedIn-65 FoundIn-66 Target-66 Target-65 FoundIn-65
Owner: sadrul@chromium.org
Status: Assigned (was: Unconfirmed)
Below is manual regression range:

Good build: 65.0.3303.0 (Revision: 526141)
Bad build: 65.0.3304.0 (Revision: 526164)

Using the per-revision bisect providing the bisect results:

You are probably looking for a change made after 526152 (known good), but no later than 526154 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/d37a9f10c7e32ff40e864a2ff30478fc8a0a1169..8e8ae37d2e02943cadb4d658777866e8e4a12f60

Suspecting: https://chromium.googlesource.com/chromium/src/+/8e8ae37d2e02943cadb4d658777866e8e4a12f60

@sadrul: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

NOTE: This issue is also reproducible on M-66 Canary (build #66.0.3334.0).

Kindly review the attached screen-cast for reference.

Thank you.
Actual_behavior.mp4
1.2 MB View Download
Expected behavior.mp4
347 KB View Download

Comment 2 by sadrul@chromium.org, Jan 30 2018

Cc: sadrul@chromium.org
Owner: kenrb@chromium.org
--> kenrb@ I believe this is already fixed. Maybe the fix needs to be merged to M65?

Comment 3 by kenrb@chromium.org, Jan 30 2018

Cc: wjmaclean@chromium.org
I don't think this is fixed. James landed a fix to the problem of the mouse cursor not being updated, but I am not aware of anything having changed wrt tooltips.

Why would this have regressed recently? Have we changed the input flow for GuestViews?

Comment 4 by sadrul@chromium.org, Jan 30 2018

I am assuming this is because the pdf (or its UI elements) are not receiving the mouse events (e.g. enter/hover) correctly.
Cc: -wjmaclean@chromium.org kenrb@chromium.org
Owner: wjmaclean@chromium.org
I'm suspecting this bug ... so far I haven't been able to detect any MouseEnter events for either PDF or OOPIFs. I'm looking into the coordinates right now to see what's going on.
Status: Started (was: Assigned)
Re c#5, I meant to say, I'm suspect this issue: https://bugs.chromium.org/p/chromium/issues/detail?id=800333
Ok, it seems like since

https://chromium.googlesource.com/chromium/src/+/8e8ae37d2e02943cadb4d658777866e8e4a12f60

landed, we no longer hit

https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?rcl=9a7a3991e35998df423168d6f550974d41944f72&l=213

anymore, and this causes the bug since we (somehow) rely on the mouse events going through the embedder for the tool tips to work.

A simple experiment: modify

https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?rcl=9a7a3991e35998df423168d6f550974d41944f72&l=204

so it only returns for non-RenderWidgetHostViewGuest cases, and suddenly the tooltips work again (but this breaks mouse cursors ...).

I'll dig further to find out why the tooltips required our going through the embedder in the past, and see if we can get rid of that requirement.
Labels: ReleaseBlock-Stable
adding RBS, please change if required.
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 31 2018

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

commit 4d39420bc8a69e9374b120df9e49828537d31f69
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Wed Jan 31 17:20:50 2018

Refer tooltip text to root view.

With the new async hit-testing code, the RenderWidgetHostViewGuest-
specific code in RenderWidgetHostInputEventRouter::FindMouseEventTarget
no longer seems to be hit, and thus mouse events are bypassing the
embedder (and hence the BrowserPlugin) for PDFs.

This CL changes RenderWidgetHostViewGuest so that it refers tooltip
text requests directly to the root view, instead of sending them to
the BrowserPlugin.

Bug:  807236 
Change-Id: Ic943e6bca70035fb5ac9198281412fbc6b7887f1
Reviewed-on: https://chromium-review.googlesource.com/894145
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533306}
[modify] https://crrev.com/4d39420bc8a69e9374b120df9e49828537d31f69/content/browser/frame_host/render_widget_host_view_guest.cc

Labels: Merge-Request-65
Status: Fixed (was: Started)
This should be fixed now in ToT.

I've marked MergeRequest-65 now to get the process started, but will let this bake a few days before attempting the merge.
Labels: TE-Verified-M66 TE-Verified-66.0.3336.0
Update:-
 Tested this issue on Windows (7,8,10), Mac(10.12.6, 10.13.1, 10.13.3) and Linux(14.04 LTS) machines using latest Chrome canary build# 66.0.3336.0 and fix is working as expected i.e. Tooltip are appears for zoom in (+), Zoom out (-) and Fit to page options.. Hence adding TE Verified labels. 

Please find the attached screen-cast for reference.

Thanks..!
Latest Canary behavior.mp4
659 KB View Download
Project Member

Comment 13 by sheriffbot@chromium.org, Feb 1 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M65 branch 3325 ASAP so we can pick it up for next M65 dev release. Thank you.
Project Member

Comment 15 by bugdroid1@chromium.org, Feb 1 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24ccf83dc018eab5f9bd1cc429b1c14da701b1e4

commit 24ccf83dc018eab5f9bd1cc429b1c14da701b1e4
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Thu Feb 01 19:32:22 2018

Refer tooltip text to root view.

With the new async hit-testing code, the RenderWidgetHostViewGuest-
specific code in RenderWidgetHostInputEventRouter::FindMouseEventTarget
no longer seems to be hit, and thus mouse events are bypassing the
embedder (and hence the BrowserPlugin) for PDFs.

This CL changes RenderWidgetHostViewGuest so that it refers tooltip
text requests directly to the root view, instead of sending them to
the BrowserPlugin.

TBR=wjmaclean@chromium.org

(cherry picked from commit 4d39420bc8a69e9374b120df9e49828537d31f69)

Bug:  807236 
Change-Id: Ic943e6bca70035fb5ac9198281412fbc6b7887f1
Reviewed-on: https://chromium-review.googlesource.com/894145
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#533306}
Reviewed-on: https://chromium-review.googlesource.com/897055
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#240}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/24ccf83dc018eab5f9bd1cc429b1c14da701b1e4/content/browser/frame_host/render_widget_host_view_guest.cc

Labels: ET-MUM-Reported
Labels: TE-Verified-M65 TE-Verified-65.0.3325.51
Update:-
 Re-tested this issue on Windows (7,8,10), Mac(10.12.6, 10.13.1, 10.13.3) and Linux(14.04 LTS) machines using Dev build# 65.0.3325.51 and fix is working as expected i.e. Tooltip is appearing for zoom in (+), Zoom out (-) and Fit to page options.. Hence adding TE-Verified labels. 

Please find the attached screen-cast for your reference.

Thanks..!
65.0.3325.51_behavior.mp4
452 KB View Download

Sign in to add a comment