-
Notifications
You must be signed in to change notification settings - Fork 16.2k
fix: window.print() only working once #21893
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
Conversation
5e53cb8
to
2382b6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Release Notes Persisted
|
I was unable to backport this PR to "8-x-y" cleanly; |
I was unable to backport this PR to "7-1-x" cleanly; |
@codebytere has manually backported this PR to "8-x-y", please check out #21908 |
@codebytere has manually backported this PR to "7-1-x", please check out #21911 |
@codebytere has manually backported this PR to "6-1-x", please check out #21913 |
Description of Change
Fixes #21397.
We should be disconnecting from the current print job for
window.print()
, unlike forwebContents.print()
. This ensures that happens by gating the disconnect call withcallback.is_null()
; the callback member is only non-null forwebContents.print()
.Tested with https://gist.github.com/0f0e776b138452d6815ef4efc16aa0d1 for the following:
webContents.print()
andwindow.print()
do not crashwebContents.print()
andwindow.print()
can be called multiple times without failingwebContents.print()
orwindow.print()
is calledwebContents.print()
when it is not called in silent modecc @deepak1556 @nornagon @zcbenz
Checklist
npm test
passesRelease Notes
Notes: Fixed an issue where
window.print()
only worked once on a singleBrowserWindow
.