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

Let finishPaintTask in pdf_page_view.js return a promise instead, to avoid having to throw in the paintTask.promise rejection handler, and don't reject the PDFPageView_draw promise when rendering is cancelled #7829

Merged
merged 1 commit into from
Dec 27, 2016

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Nov 19, 2016

As mentioned on IRC yesterday, we currently throw even when rendering is cancelled, which is annoying when the devtools are active. Furthermore, since cancelled isn't really an error, rejecting the PDFPageView_draw promise seems somewhat strange in that case.

Perhaps marginally easier reviewing with https://github.com/mozilla/pdf.js/pull/7829/files?w=1.


This change is Reviewable

@Snuffleupagus
Copy link
Collaborator Author

@yurydelendik Gentle review ping?

…o avoid having to throw in the `paintTask.promise` rejection handler, and don't reject the `PDFPageView_draw` promise when rendering is `cancelled`

As mentioned on IRC yesterday, we currently throw even when rendering is `cancelled`, which is annoying when the devtools are active. Furthermore, since `cancelled` isn't really an error, rejecting the `PDFPageView_draw` promise seems somewhat strange in that case.
@Snuffleupagus
Copy link
Collaborator Author

@timvandermeij Since I'm not sure how much time Yury has for PDF.js, is this something that you feel comfortable reviewing? Please note that "no" is a perfectly valid answered here!

For some more context, the IRC discussion mentioned above: http://logs.glob.uno/?c=mozilla%23pdfjs&s=18+Nov+2016&e=18+Nov+2016#c54305.

@timvandermeij timvandermeij requested review from timvandermeij and removed request for yurydelendik December 27, 2016 15:44
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

Live output at: http://107.21.233.14:8877/1e0d0cb7379fbe0/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/1e0d0cb7379fbe0/output.txt

Total script time: 2.22 mins

Published

@timvandermeij timvandermeij merged commit 22f0a04 into mozilla:master Dec 27, 2016
@timvandermeij
Copy link
Contributor

Thank you for the patch!

@Snuffleupagus Snuffleupagus deleted the finishPaintTask-promise branch December 28, 2016 09:10
@Snuffleupagus
Copy link
Collaborator Author

@timvandermeij Thanks so much for landing this; the throwing on cancelled was really annoying me!

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

Let `finishPaintTask` in pdf_page_view.js return a promise instead, to avoid having to throw in the `paintTask.promise` rejection handler, and don't reject the `PDFPageView_draw` promise when rendering is `cancelled`
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.

4 participants