Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make print() run synchronously #4690

Merged
merged 1 commit into from
Jun 24, 2019
Merged

Make print() run synchronously #4690

merged 1 commit into from
Jun 24, 2019

Conversation

TimothyGu
Copy link
Member

@TimothyGu TimothyGu commented Jun 12, 2019

This was a slip-up in the larger clean-up commit in 61da493, where "synchronous" was changed to "in parallel". All browsers implement the synchronous behavior.

Fixes #4686.


/timers-and-user-prompts.html ( diff )

This was a slip-up in the larger clean-up commit in 61da493, where
"synchronous" was changed to "in parallel". All browsers implement the
synchronous behavior.

Fixes whatwg#4686.
@TimothyGu
Copy link
Member Author

I’m not sure quite to test this by the way, nor does there seem to be any existing tests for print() in WPT.

@domenic
Copy link
Member

domenic commented Jun 12, 2019

I think you could test at least beforeprint by calling preventDefault. Testing afterprint would require a manual test so that someone could cancel out of the print dialog. /cc @gsnedders in case he knows things about testing print dialogs.

@domenic
Copy link
Member

domenic commented Jun 12, 2019

We should also, from https://html.spec.whatwg.org/multipage/parsing.html#the-end:print-when-loaded, queue a task to run the printing steps, instead of running them directly. Right?

@TimothyGu
Copy link
Member Author

I think you could test at least beforeprint by calling preventDefault.

This doesn't seem to work, as printing steps don't respect preventDefault either in spec or in implementations.

We should also, from https://html.spec.whatwg.org/multipage/parsing.html#the-end:print-when-loaded, queue a task to run the printing steps, instead of running them directly. Right?

I think you are right from some light testing, though that seems to be an existing bug. In particular, the following

<script>
  onpageshow = () => { console.log('pageshow', document.readyState); };
  onload = () => { console.log('load', document.readyState); };
  onbeforeprint = (e) => { console.log('beforeprint', document.readyState); e.preventDefault(); };
  print();
  console.log('after print() returns');
</script>

prints

after print() returns
load complete
pageshow complete
beforeprint complete

in all the browsers I can test.

@domenic
Copy link
Member

domenic commented Jun 12, 2019

Sorry, I forgot the purpose of beforeprint; you're right, it's not cancelable.

So yeah, I'm happy to land this when we fix the preexisting bug, plus file an issue for adding manual tests and/or figuring out how to automate them.

@gsnedders
Copy link

Pretty sure there's no way to test that it happens synchronously.

@zcorpan
Copy link
Member

zcorpan commented Jun 13, 2019

Please make sure there's an issue in wpt with label type:untestable when you run into things that can't be automated, and document why you need it.

This issue seems related at least web-platform-tests/wpt#5667

@TimothyGu TimothyGu added the needs tests Moving the issue forward requires someone to write tests label Jun 14, 2019
@TimothyGu TimothyGu removed the needs tests Moving the issue forward requires someone to write tests label Jun 23, 2019
@TimothyGu
Copy link
Member Author

Test added in web-platform-tests/wpt#17439.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM, with tests in progress. Let's merge this.

@domenic domenic merged commit d89b68a into whatwg:master Jun 24, 2019
@TimothyGu TimothyGu deleted the printing branch June 24, 2019 14:31
domenic pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 2, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 24, 2019
Automatic update from web-platform-tests
Add manual test for print()

For whatwg/html#4690.
--

wpt-commits: 8e53d2a89fe610b2ce480877c60bca88c949cf60
wpt-pr: 17439
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Jul 25, 2019
Automatic update from web-platform-tests
Add manual test for print()

For whatwg/html#4690.
--

wpt-commits: 8e53d2a89fe610b2ce480877c60bca88c949cf60
wpt-pr: 17439
natechapin pushed a commit to natechapin/wpt that referenced this pull request Aug 23, 2019
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
Automatic update from web-platform-tests
Add manual test for print()

For whatwg/html#4690.
--

wpt-commits: 8e53d2a89fe610b2ce480877c60bca88c949cf60
wpt-pr: 17439

UltraBlame original commit: 3c43e31a85a2a98871d88f17af5b6b9f31f32616
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
Automatic update from web-platform-tests
Add manual test for print()

For whatwg/html#4690.
--

wpt-commits: 8e53d2a89fe610b2ce480877c60bca88c949cf60
wpt-pr: 17439

UltraBlame original commit: 3c43e31a85a2a98871d88f17af5b6b9f31f32616
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
Automatic update from web-platform-tests
Add manual test for print()

For whatwg/html#4690.
--

wpt-commits: 8e53d2a89fe610b2ce480877c60bca88c949cf60
wpt-pr: 17439

UltraBlame original commit: 3c43e31a85a2a98871d88f17af5b6b9f31f32616
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

"Printing steps" should not run in parallel
4 participants