Bug 84181 - DevTools highlights elements in frames at un-scaled positions
Summary: DevTools highlights elements in frames at un-scaled positions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Xianzhu Wang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-17 12:31 PDT by Xianzhu Wang
Modified: 2012-04-20 14:05 PDT (History)
4 users (show)

See Also:


Attachments
patch (9.05 KB, patch)
2012-04-17 14:17 PDT, Xianzhu Wang
pfeldman: review-
pfeldman: commit-queue-
Details | Formatted Diff | Diff
patch v2 (added comments in ChangeLog) (9.63 KB, patch)
2012-04-18 10:03 PDT, Xianzhu Wang
pfeldman: review+
Details | Formatted Diff | Diff
patch for landing (9.60 KB, patch)
2012-04-19 09:54 PDT, Xianzhu Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xianzhu Wang 2012-04-17 12:31:47 PDT
Reproduced on chromium-android (with page scaling).

1. On device, open www.gmail.com and login and switch to desktop version
2. start DevTools on the host to inspect the gmail page
3. In DevTools Elements pane, hover any element in the "canvas_frame" iframe.

Observed behavior:
The highlighted position is not the actual position of the hovered element, but the un-scaled position

Expected Behavior:
Should highlight the correct position of the element

Will upload a patch soon.
Comment 1 Xianzhu Wang 2012-04-17 14:17:25 PDT
Created attachment 137600 [details]
patch
Comment 2 Pavel Feldman 2012-04-18 05:34:45 PDT
Comment on attachment 137600 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137600&action=review

> Source/WebCore/ChangeLog:8
> +        Test: inspector/elements/highlight-node-scaled.html

Please explain what changed and why in the ChangeLog. I can't reproduce it on ToT Chromium. Also, I am not sure what you expect to "scale" in a given test case.
Comment 3 Xianzhu Wang 2012-04-18 10:03:47 PDT
Created attachment 137715 [details]
patch v2 (added comments in ChangeLog)

(In reply to comment #2)
> Please explain what changed and why in the ChangeLog. I can't reproduce it on ToT Chromium. Also, I am not sure what you expect to "scale" in a given test case.

Sorry for not being clear in the last patch. Added comments in ChangeLog in this patch.

The scaling feature is used on some mobile platforms, e.g. chromium-android. However, the issue can be reproduced on all platforms with the new test case which forces scaling with window.internals.settings.setPageScale(2, 0, 0). The change uses FrameView::contentsToRootView() to map the coordinates of a node in a frame to the coordinates in the main view, instead of calculating by only the offsets.
Comment 4 Pavel Feldman 2012-04-19 03:59:31 PDT
Comment on attachment 137715 [details]
patch v2 (added comments in ChangeLog)

View in context: https://bugs.webkit.org/attachment.cgi?id=137715&action=review

> LayoutTests/inspector/elements/highlight-node-scaled.html:39
> +    window.internals.settings.setPageScaleFactor(2, 0, 0);

This should be:
if (window.internals)
    window.internals.settings.setPageScaleFactor(2, 0, 0);

So that the test could be loaded outside DRT.
Comment 5 Xianzhu Wang 2012-04-19 09:54:15 PDT
Comment on attachment 137715 [details]
patch v2 (added comments in ChangeLog)

View in context: https://bugs.webkit.org/attachment.cgi?id=137715&action=review

>> LayoutTests/inspector/elements/highlight-node-scaled.html:39
>> +    window.internals.settings.setPageScaleFactor(2, 0, 0);
> 
> This should be:
> if (window.internals)
>     window.internals.settings.setPageScaleFactor(2, 0, 0);
> 
> So that the test could be loaded outside DRT.

Done. Thanks!
Comment 6 Xianzhu Wang 2012-04-19 09:54:51 PDT
Created attachment 137913 [details]
patch for landing
Comment 7 Pavel Feldman 2012-04-19 09:58:42 PDT
Comment on attachment 137913 [details]
patch for landing

Thanks.
Comment 8 WebKit Review Bot 2012-04-19 11:42:18 PDT
Comment on attachment 137913 [details]
patch for landing

Clearing flags on attachment: 137913

Committed r114659: <http://trac.webkit.org/changeset/114659>
Comment 9 WebKit Review Bot 2012-04-19 11:42:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Jer Noble 2012-04-20 14:05:23 PDT
This changeset is causing a crash in inspector/elements/elements-panel-selection-on-refresh.html.

Filed https://bugs.webkit.org/show_bug.cgi?id=84492 to track the new crash.