Inspect on embedded <webview> gets incorrect element
Reported by
pett...@vivaldi.com,
Jan 4 2018
|
||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.97 Safari/537.36 Vivaldi/1.95.1049.1 Steps to reproduce the problem: 1. Unpack the attached chrome app and launch it. 2. Right click any element on the embedded page in the <webview>. 3. Notice the wrong element is selected for inspect. What is the expected behavior? Correct element is selected for inspect. What went wrong? It appears that the issue is located in RenderFrameHostImpl::OnContextMenu() where a call to TransformPointToRootCoordSpace() is made which translated the coordinates from being relative to the <webview> to be relative to the embedder. Did this work before? Yes 62.0.3202.97 Does this work in other browsers? N/A Chrome version: 64.0.3282.71 Channel: beta OS Version: 10.0 Flash Version: This seems to also be broken in 63, and 64. I did not find a way to check 65 due to the new UI that do not have an explicit way to launch the app anymore.
,
Jan 4 2018
,
Jan 5 2018
pettern@ Thanks for the issue. Tested this issue on Windows 10 and Mac OS 10.12.6 on the reported version 62.0.3202.97, the latest Stable 63.0.3239.132 and Canary 65.0.3312.0 by following the below steps. 1. Launched Chrome and navigated to chrome://extensions. 2. Added the given extension and launched it. 3. Right-clicked on the page and selected Inspect. 4. In the Elements tab, on hovering the mouse, can see the elements are selected. Attached is the screen cast for reference. Request you to please check and confirm if anything is missed from our end in testing this issue. Also request you to please provide a screen cast for better understanding of the issue. Thanks..
,
Jan 5 2018
Yes, the crucial step you're missing is that when right clicking a specific element and selecting Inspect, that element selected to inspect is not the active element in devtools. I misses the element and does inspect on a random element further away.
,
Jan 5 2018
Thank you for providing more feedback. Adding requester "sc00335628@techmahindra.com" to the cc list and removing "Needs-Feedback" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 8 2018
pettern@ Thanks for the feedback. Able to reproduce this issue on Windows 10, Mac OS 10.12.6 and Ubuntu 14.04 on the latest Canary 65.0.3315.0 and Stable 63.0.3239.132. Bisect Information: ==================== Good Build: 63.0.3223.0 (Revision - 503964) Bad Build : 63.0.3225.0 (Revision - 504540) On executing the per-revision bisect script, below is the changelog URL. Changelog URL: --------------- https://chromium.googlesource.com/chromium/src/+log/1b62135c07abbd0fb531b03fddb3c2114cb06698..3131414035a033d1fb624913474dbdac04158198 From the above Changelog URL, suspecting the below change for this issue. Reviewed-on: https://chromium-review.googlesource.com/680158 lfg@ Can you please check if this issue is related to your change, else help us in assigning to the right owner. Adding label ReleaseBlock-Stable as this is a recent regression. Please feel free to remove the same if it is not applicable. Thanks...
,
Jan 8 2018
This has been going for a few releases so I'm removing RBS. I'll put a fix soon, but won't be merging.
,
Jan 8 2018
Just to add to the previous comment, this is a minor issue, it only affects devtools and we shouldn't block rollout based on this alone.
,
Jan 8 2018
Since the CL just enabled the finch feature by default in M63 and this feature was enabled since M61. Please target the fix on M64.
,
Jan 9 2018
Since this is Blink related change tagging ChromeOS and Android.
,
Jan 9 2018
,
Jan 9 2018
,
Jan 9 2018
This isn't a stable blocker. And M63 is done with stable updates, so fix should be in 64 beta.
,
Jan 9 2018
<webview> tag is not supported on Android, just desktop.
,
Jan 10 2018
As per the above comments (#8 & #9) marking this as "Release blocker for M64 Stable". I shouldn't think this is a Beta blocker for M64.
,
Jan 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cc7abbc3189248c4dd2698c71bf692abc97b40d commit 9cc7abbc3189248c4dd2698c71bf692abc97b40d Author: Lucas Gadani <lfg@chromium.org> Date: Wed Jan 10 22:57:57 2018 Transform mouse coordinates when inspecting inner WebContents. Bug: 798934 Change-Id: I2b344466c96eb987cd5cddaf1c28c8ccd126291e Reviewed-on: https://chromium-review.googlesource.com/854996 Commit-Queue: Lucas Gadani <lfg@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/heads/master@{#528456} [modify] https://crrev.com/9cc7abbc3189248c4dd2698c71bf692abc97b40d/content/browser/devtools/render_frame_devtools_agent_host.cc [modify] https://crrev.com/9cc7abbc3189248c4dd2698c71bf692abc97b40d/content/browser/renderer_host/render_widget_host_view_child_frame.cc [modify] https://crrev.com/9cc7abbc3189248c4dd2698c71bf692abc97b40d/content/browser/renderer_host/render_widget_host_view_child_frame.h
,
Jan 11 2018
Verified the fix on Mac 10.12.6, Win-10 and Ubuntu 14.04 using latest Chrome version #65.0.3318.0 as per the comment #0 & #4. Attaching screen cast for reference. Observed that correct element is selected for inspect. Hence, the fix is working as expected. Adding the verified labels. Thanks...!!
,
Jan 11 2018
,
Jan 11 2018
,
Jan 11 2018
This bug requires manual review: We are only 11 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 11 2018
Thanks for the fix lfg@ - seems like it's already been verified, but can you confirm this is a safe fix/merge overall, and no chances of introducing new regressions? We're only 11 days away from Stable.
,
Jan 11 2018
I believe this is a low-risk merge and it fixes a regression. Also adding some more people who may have thoughts on this.
,
Jan 11 2018
I would give this one more day on Canary, but the fix was pretty isolated.
,
Jan 16 2018
Thanks - approving merge for M64. Branch:3282
,
Jan 16 2018
I just tried to merge the CL back to M64, but there was one large refactor between the branch and my change (r526758), which makes it harder and riskier to merge. Given that we are very close to stable promotion for M64, I don't think there's sufficient time to backport the change and bake it into M64, so even though this is a regression I've decided not to merge the fix back.
,
Jan 16 2018
Thanks for not merging. Let's target this for M65. |
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by pett...@vivaldi.com
, Jan 4 2018