Log an error when the value passed to set currentPageNumber is out of bounds (PR 7502 followup)
#7529
Conversation
| }, | ||
|
|
||
| get page() { // TODO remove | ||
| return this.pdfLinkService.page; | ||
| return this.pdfViewer.currentPageNumber; |
There was a problem hiding this comment.
Now that this does not use the link service anymore, do we still need the TODO remove comment one line above? I don't see why this getter needs to go, and otherwise I think the setter would have to go as well.
There was a problem hiding this comment.
Yeah, I'm not entirely sure why that comment is there in the first place, and we probably should keep the page getter/setter around for backwards compatibility.
I'll just remove the comment; thanks for pointing this out!
…of bounds (PR 7502 followup) With PR 7502 we no longer dispatch an event when the `val` is out of bounds, so to better communicate why nothing happens this patch logs an error in that case (similar to the logging of errors when trying to set an invalid scale). The way that the default viewer is currently implemented, means that e.g. keyboard short-cuts could trigger the new error. Hence this patch also adds the necessary validation code, both to `app.js` and `pdf_link_service.js` to prevent unnecessary errors.
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/3c656244020b94b/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/3c656244020b94b/output.txt Total script time: 1.02 mins Published |
|
Thank you for working on this! |
|
@timvandermeij And thank you for reviewing this, and other PRs! |
With PR #7502 we no longer dispatch an event when the
valis out of bounds, so to better communicate why nothing happens this patch logs an error in that case (similar to the logging of errors when trying to set an invalid scale).The way that the default viewer is currently implemented, means that e.g. keyboard short-cuts could trigger the new error. Hence this patch also adds the necessary validation code, both to
app.jsandpdf_link_service.jsto prevent unnecessary errors.Note: perhaps slightly simpler reviewing with https://github.com/mozilla/pdf.js/pull/7529/files?w=1.