Bug 222920 - Web Inspector: Grid overlay does not adjust for element inside iframes
Summary: Web Inspector: Grid overlay does not adjust for element inside iframes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Patrick Angle
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-08 10:23 PST by Patrick Angle
Modified: 2021-03-15 21:33 PDT (History)
8 users (show)

See Also:

Attachments
Patch v1.0 (12.71 KB, patch)
2021-03-15 11:39 PDT, Patrick Angle 		no flags Details | Formatted Diff | Diff
Video of Patch v1.0 (47.15 MB, video/quicktime)
2021-03-15 11:41 PDT, Patrick Angle 		no flags Details
Patch v1.1 - Maintain existing integer precision paths for compatibility (15.36 KB, patch)
2021-03-15 15:01 PDT, Patrick Angle 		no flags Details | Formatted Diff | Diff
Patch v1.2 - Use enum instead of bool for precision parameter (16.08 KB, patch)
2021-03-15 15:38 PDT, Patrick Angle 		no flags Details | Formatted Diff | Diff
Patch v1.3 - Redux of v1.0 for EWS (10.85 KB, patch)
2021-03-15 16:40 PDT, Patrick Angle 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Angle 2021-03-08 10:23:27 PST 
Grid overlay do not currently account for being inside an iframe when drawing, so gridlines can appear offset from the grid itself.
Comment 1 Radar WebKit Bug Importer 2021-03-08 16:13:21 PST 
<rdar://problem/75192511>
Comment 2 Patrick Angle 2021-03-15 11:39:43 PDT 
Created attachment 423209 [details]
Patch v1.0
Comment 3 Patrick Angle 2021-03-15 11:41:20 PDT 
Created attachment 423210 [details]
Video of Patch v1.0
Comment 4 BJ Burg 2021-03-15 11:50:59 PDT 
Comment on attachment 423209 [details]
Patch v1.0

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

Overlay changes look correct, but I have concerns about changing ScrollView/FrameView helpers.

> Source/WebCore/page/FrameView.cpp:4791
> +

Nit: remove extra newline or inline the entire expression.

> Source/WebCore/platform/ScrollView.cpp:894
> +    return point - toFloatSize(documentScrollPositionRelativeToViewOrigin());

Please don't change this here, it's used elsewhere and I have no confidence that it's okay to do this in all cases.

Alternatively, add an optional argument that opts into the new behavior, while leaving existing callsites and behavior unchanged.

> Source/WebCore/platform/Widget.cpp:266
> +    if (const ScrollView* parentScrollView = parent())

Ditto above comment about changing behaviors.
Comment 5 Patrick Angle 2021-03-15 15:01:33 PDT 
Created attachment 423247 [details]
Patch v1.1 - Maintain existing integer precision paths for compatibility
Comment 6 Patrick Angle 2021-03-15 15:38:58 PDT 
Created attachment 423254 [details]
Patch v1.2 - Use enum instead of bool for precision parameter
Comment 7 Patrick Angle 2021-03-15 16:40:32 PDT 
Created attachment 423261 [details]
Patch v1.3 - Redux of v1.0 for EWS
Comment 8 EWS 2021-03-15 21:33:36 PDT 
Committed r274464: <https://commits.webkit.org/r274464>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423261 [details].