Bug 238060

Summary: Web Inspector: `console.screenshot` has extra transparent pixels at bottom of screenshot on platforms with viewports that are obscured by browser chrome
Product: WebKit Reporter: Patrick Angle <pangle>
Component: Web InspectorAssignee: Patrick Angle <pangle>
Status: RESOLVED FIXED    
Severity: Normal CC: hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Screenshot of Issue
none
Patch v1.0
none
Screenshot of Patch v1.0
none
Patch v1.1 none

Description Patrick Angle 2022-03-17 17:04:08 PDT
.
Comment 1 Radar WebKit Bug Importer 2022-03-17 17:04:19 PDT
<rdar://problem/90460824>
Comment 2 Patrick Angle 2022-03-17 17:04:26 PDT
Created attachment 455046 [details]
Screenshot of Issue
Comment 3 Patrick Angle 2022-03-17 17:08:19 PDT
Created attachment 455049 [details]
Patch v1.0
Comment 4 Patrick Angle 2022-03-17 17:08:44 PDT
Created attachment 455050 [details]
Screenshot of Patch v1.0
Comment 5 Devin Rousso 2022-03-17 17:17:39 PDT
Comment on attachment 455049 [details]
Patch v1.0

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

r=me, lol oops great catch

Any way to test this?

> Source/WebCore/page/PageConsoleClient.cpp:414
> +            IntRect imageRect(IntPoint::zero(), m_page.mainFrame().view()->unobscuredContentRect().size());

Do we care about the `location()` of the `unobscuredContentRect()`?  Should we maybe just use that as the `imageRect` instead of only taking it's `size()` with `IntPoint::zero()` as the `location`?
Comment 6 Patrick Angle 2022-03-17 17:22:27 PDT
Comment on attachment 455049 [details]
Patch v1.0

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

>> Source/WebCore/page/PageConsoleClient.cpp:414
>> +            IntRect imageRect(IntPoint::zero(), m_page.mainFrame().view()->unobscuredContentRect().size());
> 
> Do we care about the `location()` of the `unobscuredContentRect()`?  Should we maybe just use that as the `imageRect` instead of only taking it's `size()` with `IntPoint::zero()` as the `location`?

The implementation of `unobscuredContentRect()` sets the `location()` to the current scroll position, which we don't want... although I guess we could stop passing `SnapshotFlags::InViewCoordinates` into `snapshotFrameRect` below and that would work (I think).
Comment 7 Patrick Angle 2022-03-18 13:11:12 PDT
Created attachment 455128 [details]
Patch v1.1
Comment 8 EWS 2022-03-18 20:26:48 PDT
Committed r291519 (248625@main): <https://commits.webkit.org/248625@main>

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