Bug 238060 - Web Inspector: `console.screenshot` has extra transparent pixels at bottom of screenshot on platforms with viewports that are obscured by browser chrome
Summary: Web Inspector: `console.screenshot` has extra transparent pixels at bottom of...
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: 2022-03-17 17:04 PDT by Patrick Angle
Modified: 2022-03-18 20:26 PDT (History)
3 users (show)

See Also:

Attachments
Screenshot of Issue (457.60 KB, image/png)
2022-03-17 17:04 PDT, Patrick Angle 		no flags Details
Patch v1.0 (2.33 KB, patch)
2022-03-17 17:08 PDT, Patrick Angle 		no flags Details | Formatted Diff | Diff
Screenshot of Patch v1.0 (423.50 KB, image/png)
2022-03-17 17:08 PDT, Patrick Angle 		no flags Details
Patch v1.1 (2.45 KB, patch)
2022-03-18 13:11 PDT, Patrick Angle 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 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].