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

Issue 798934 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Inspect on embedded <webview> gets incorrect element

Reported by pett...@vivaldi.com, Jan 4 2018

Issue description

UserAgent: 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.
 
ChromeApp.zip
1.1 KB Download
Labels: Needs-Bisect Needs-Triage-M63
Cc: sc00335...@techmahindra.com
Labels: Triaged-ET Needs-Feedback
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..

798934.webm
9.8 MB View Download
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. 
Project Member

Comment 5 by sheriffbot@chromium.org, Jan 5 2018

Labels: -Needs-Feedback
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
Cc: ranjitkan@chromium.org pbomm...@chromium.org
Labels: -Pri-2 -Needs-Bisect hasbisect-per-revision ReleaseBlock-Stable M-63 OS-Linux OS-Mac Pri-1
Owner: lfg@chromium.org
Status: Assigned (was: Unconfirmed)
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...

Comment 7 by lfg@chromium.org, Jan 8 2018

Labels: -ReleaseBlock-Stable
This has been going for a few releases so I'm removing RBS. I'll put a fix soon, but won't be merging.

Comment 8 by lfg@chromium.org, 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.

Labels: ReleaseBlock-Stable M-64 FoundIn-64 FoundIn-65 FoundIn-63 Target-64
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.
Labels: OS-Android OS-Chrome
Since this is Blink related change tagging ChromeOS and Android.
Labels: RegressedIn-63 Target-65
Labels: -ReleaseBlock-Stable -M-63 ReleaseBlock-Beta
This isn't a stable blocker. And M63 is done with stable updates, so fix should be in 64 beta.

Comment 14 by lfg@chromium.org, Jan 9 2018

Labels: -OS-Android
<webview> tag is not supported on Android, just desktop.

Labels: -ReleaseBlock-Beta ReleaseBlock-Stable
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.
Project Member

Comment 16 by bugdroid1@chromium.org, 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

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...!!
798934.mp4
2.4 MB View Download
Cc: viswatej...@techmahindra.com
Labels: TE-Verified-M65 TE-Verified-65.0.3318.0

Comment 19 by lfg@chromium.org, Jan 11 2018

Labels: Merge-Request-64
Status: Fixed (was: Assigned)
Project Member

Comment 20 by sheriffbot@chromium.org, Jan 11 2018

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
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
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. 

Comment 22 by lfg@chromium.org, Jan 11 2018

Cc: kenrb@chromium.org dgozman@chromium.org
I believe this is a low-risk merge and it fixes a regression.

Also adding some more people who may have thoughts on this.

I would give this one more day on Canary, but the fix was pretty isolated.
Labels: -Merge-Review-64 Merge-Approved-64
Thanks - approving merge for M64. Branch:3282

Comment 25 by lfg@chromium.org, Jan 16 2018

Cc: abdulsyed@chromium.org
Labels: -Hotlist-Merge-Review -Merge-Approved-64
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.

Thanks for not merging. Let's target this for M65. 

Sign in to add a comment