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

Printing refactor split from 7721 #7766

Merged
merged 4 commits into from
Oct 31, 2016

Conversation

Rob--W
Copy link
Member

@Rob--W Rob--W commented Oct 30, 2016

This PR contains all reviewed commits from #7721 (without changes) and fixes #7720.
Maybe the complex OOP printing patch from the other PR is not needed if Chrome fixes the underlying bug before Chrome-with-the-bug reaches the stable channel.

Make sure that the print service is stopped as soon as possible when
aborted, and that it is not possible for a (slow) promise to
accidentally wipe the state of a print job that was started later.
- Move the global scratchCanvas to PDFPrintService. This is mainly to
  make it easier to reason about the state of scratchCanvas. In practice
  there is no difference because only one PDFPrintService instance can
  be instantiated at any given time.

- Move all logic of using the rendered page to one location.
  This makes it easier to replace the printing logic later, when I add
  special handling to out-of-process frames in the Chrome extension.
- Renamed startPrint to performPrint to emphasize that the method
  does not start the print process (preparing pages for the printer),
  but that it does the actual printing (sending pages off to the
  printer).

- Put performPrint in the PDFPrintService, so that it can be
  overridden if needed.
@Rob--W
Copy link
Member Author

Rob--W commented Oct 30, 2016

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @Rob--W received. Current queue size: 0

Live output at: http://107.22.172.223:8877/cd0769c6c273abd/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/cd0769c6c273abd/output.txt

Total script time: 3.48 mins

Published

@Rob--W
Copy link
Member Author

Rob--W commented Oct 30, 2016

@yurydelendik You've already reviewed the patches, please merge if you think that it's OK to merge.

Only the first two patches are necessary to fix #7720, the other two only make it easier to extend the PDF printing logic later, if wanted.

@yurydelendik
Copy link
Contributor

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/2fa8c31ff31be3f/output.txt

@yurydelendik yurydelendik merged commit a740d69 into mozilla:master Oct 31, 2016
@yurydelendik
Copy link
Contributor

Thank you for the patch

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/2fa8c31ff31be3f/output.txt

Total script time: 3.29 mins

Published

movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018
…rom-7721

Printing refactor split from 7721
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.

Print before document is ready causes an alert dialog + print overlay is stuck
3 participants