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 descriptionChrome 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.
,
Jan 30 2018
--> kenrb@ I believe this is already fixed. Maybe the fix needs to be merged to M65?
,
Jan 30 2018
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?
,
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.
,
Jan 30 2018
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.
,
Jan 30 2018
,
Jan 30 2018
Re c#5, I meant to say, I'm suspect this issue: https://bugs.chromium.org/p/chromium/issues/detail?id=800333
,
Jan 30 2018
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.
,
Jan 31 2018
adding RBS, please change if required.
,
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
,
Jan 31 2018
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.
,
Feb 1 2018
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..!
,
Feb 1 2018
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
,
Feb 1 2018
Pls merge your change to M65 branch 3325 ASAP so we can pick it up for next M65 dev release. Thank you.
,
Feb 1 2018
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
,
Feb 2 2018
,
Feb 6 2018
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..! |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by dchau...@etouch.net
, Jan 30 2018Owner: sadrul@chromium.org
Status: Assigned (was: Unconfirmed)
1.2 MB
1.2 MB View Download
347 KB
347 KB View Download