Skip to content

Add "document" screenshots#564

Merged
jrandolf-google merged 1 commit intomainfrom
jrandolf/document-screenshot
Oct 12, 2023
Merged

Add "document" screenshots#564
jrandolf-google merged 1 commit intomainfrom
jrandolf/document-screenshot

Conversation

@jrandolf-google
Copy link
Contributor

@jrandolf-google jrandolf-google commented Sep 29, 2023

This PR implements document screenshots (formerly called "fullscreen screenshots").

Issue #384


Preview | Diff

@jrandolf-google jrandolf-google changed the base branch from main to jrandolf/formats September 29, 2023 12:01
@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch from b68cab9 to bc0e064 Compare September 29, 2023 12:07
@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch 2 times, most recently from 8f6d9f5 to 92db245 Compare September 29, 2023 12:10
@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch 2 times, most recently from 6f504b0 to fb333b9 Compare September 29, 2023 12:15
Copy link
Member

@thiagowfx thiagowfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM(style)

@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch from fb333b9 to 7017856 Compare October 4, 2023 07:25
@jrandolf-google jrandolf-google force-pushed the jrandolf/formats branch 3 times, most recently from 08a647e to dd89c4a Compare October 4, 2023 11:38
Base automatically changed from jrandolf/formats to main October 5, 2023 11:49
@OrKoN
Copy link
Contributor

OrKoN commented Oct 6, 2023

@jrandolf could you please resolve conflicts?

@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch from 7017856 to 71100de Compare October 6, 2023 07:30
@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch from 71100de to 861443c Compare October 6, 2023 15:04
@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch 2 times, most recently from c09b525 to 2d0aa39 Compare October 9, 2023 11:55
@OrKoN OrKoN requested a review from sadym-chromium October 9, 2023 11:56
Copy link
Member

@jgraham jgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the origin is set to viewport the clip coordinates should also be understood as viewport relative rather than document relative, and so need to be converted into document coordinates for the rest of the algorithm.

@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch from 2d0aa39 to 3d08940 Compare October 9, 2023 14:53
@jrandolf-google jrandolf-google force-pushed the jrandolf/document-screenshot branch 2 times, most recently from 3d08940 to e05f5d6 Compare October 9, 2023 15:01
@jrandolf-google jrandolf-google merged commit a3a09be into main Oct 12, 2023
@jrandolf-google jrandolf-google deleted the jrandolf/document-screenshot branch October 12, 2023 10:33
github-actions bot added a commit that referenced this pull request Oct 12, 2023
SHA: a3a09be
Reason: push, by jrandolf

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@OrKoN
Copy link
Contributor

OrKoN commented Oct 12, 2023

@jrandolf please add WPT tests for this (and also for formats)

@whimboo
Copy link
Contributor

whimboo commented Oct 26, 2023

@jrandolf please add WPT tests for this (and also for formats)

We will take care of fullpage screenshot tests over on https://bugzilla.mozilla.org/show_bug.cgi?id=1840999.

@whimboo whimboo added needs-tests module-browsingContext Browsing Context module labels Oct 26, 2023
@jrandolf-google
Copy link
Contributor Author

Oh, I've created the PR for the tests here: web-platform-tests/wpt#42804

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants