Prevent destinations with bad left/top values from scrolling the wrong page into view (bug 874482) #7440
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g page into view (bug 874482) There are PDF generators which create destinations with e.g. too large top values, which cause the wrong page to be scrolled into view because the offset becomes negative. By ignoring negative offsets, we can prevent this issue, and get a similar behaviour as in Adobe Reader. However, since we're also using `PDFViewer_scrollPageIntoView` in more cases than just when links (in the document/outline) are clicked, the patch adds a way to allow the caller to opt-out of this behaviour. In e.g. the following situations, I think that we still want to be able to allow negative offsets: when restoring a position from the `ViewHistory`, when the `viewBookmark` button is used to obtain a link to the current position, or when maintaining the current position on zooming. Rather than adding another parameter to `PDFViewer_scrollPageIntoView`, I've changed the signature to take an parameter object instead. To maintain backwards compatibility, I've added fallback code enclosed in a `GENERIC` preprocessor tag. Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=874482.
|
/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/79957b4e77888c3/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/79957b4e77888c3/output.txt Total script time: 1.07 mins Published |
|
Thank you for fixing this, @Snuffleupagus and @xlc! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are PDF generators which create destinations with e.g. too large top values, which cause the wrong page to be scrolled into view because the offset becomes negative.
By ignoring negative offsets, we can prevent this issue, and get a similar behaviour as in Adobe Reader.
However, since we're also using
PDFViewer_scrollPageIntoViewin more cases than just when links (in the document/outline) are clicked, the patch adds a way to allow the caller to opt-out of this behaviour.In e.g. the following situations, I think that we still want to be able to allow negative offsets: when restoring a position from the
ViewHistory, when theviewBookmarkbutton is used to obtain a link to the current position, or when maintaining the current position on zooming.Rather than adding another parameter to
PDFViewer_scrollPageIntoView, I've changed the signature to take an parameter object instead. To maintain backwards compatibility, I've added fallback code enclosed in aGENERICpreprocessor tag.Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=874482.
Note: This PR includes the patch from PR #6903 which is enough to fixes the bug, but with the addition of the second patch we also avoid the issues described in that PR.
/cc @timvandermeij