Prevent destinations with bad left/top values from scrolling the wrong 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.
This commit is contained in:
Jonas Jenwald 2016-06-20 12:30:05 +02:00
parent 6cb77d6580
commit 076e25f1ca
4 changed files with 57 additions and 18 deletions

View File

@ -1187,7 +1187,7 @@ var PDFViewerApplication = {
this.forceRendering();
this.pdfViewer.scrollPageIntoView(pageNumber);
this.pdfViewer.currentPageNumber = pageNumber;
},
requestPresentationMode: function pdfViewRequestPresentationMode() {

View File

@ -308,7 +308,7 @@ var PDFFindController = (function PDFFindControllerClosure() {
// If the page is selected, scroll the page into view, which triggers
// rendering the page, which adds the textLayer. Once the textLayer is
// build, it will scroll onto the selected match.
this.pdfViewer.scrollPageIntoView(index + 1);
this.pdfViewer.currentPageNumber = index + 1;
}
var page = this.pdfViewer.getPageView(index);

View File

@ -115,7 +115,10 @@ var PDFLinkService = (function PDFLinkServiceClosure() {
'Trying to navigate to a non-existent page.');
return;
}
self.pdfViewer.scrollPageIntoView(pageNumber, dest);
self.pdfViewer.scrollPageIntoView({
pageNumber: pageNumber,
destArray: dest,
});
if (self.pdfHistory) {
// Update the browsing history.
@ -241,7 +244,11 @@ var PDFLinkService = (function PDFLinkServiceClosure() {
}
}
if (dest) {
this.pdfViewer.scrollPageIntoView(pageNumber || this.page, dest);
this.pdfViewer.scrollPageIntoView({
pageNumber: pageNumber || this.page,
destArray: dest,
allowNegativeOffset: true,
});
} else if (pageNumber) {
this.page = pageNumber; // simple page
}

View File

@ -459,7 +459,11 @@ var PDFViewer = (function pdfViewer() {
dest = [null, { name: 'XYZ' }, this._location.left,
this._location.top, null];
}
this.scrollPageIntoView(page, dest);
this.scrollPageIntoView({
pageNumber: page,
destArray: dest,
allowNegativeOffset: true,
});
}
this._setScaleDispatchEvent(newScale, newValue, preset);
@ -532,16 +536,38 @@ var PDFViewer = (function pdfViewer() {
},
/**
* Scrolls page into view.
* @param {number} pageNumber
* @param {Array} dest - (optional) original PDF destination array:
* <page-ref> </XYZ|FitXXX> <args..>
* @typedef ScrollPageIntoViewParameters
* @param {number} pageNumber - The page number.
* @param {Array} destArray - (optional) The original PDF destination array,
* in the format: <page-ref> </XYZ|/FitXXX> <args..>
* @param {boolean} allowNegativeOffset - (optional) Allow negative page
* offsets. The default value is `false`.
*/
scrollPageIntoView: function PDFViewer_scrollPageIntoView(pageNumber,
dest) {
/**
* Scrolls page into view.
* @param {ScrollPageIntoViewParameters} params
*/
scrollPageIntoView: function PDFViewer_scrollPageIntoView(params) {
if (!this.pdfDocument) {
return;
}
//#if GENERIC
if (arguments.length > 1 || typeof params === 'number') {
console.warn('Call of scrollPageIntoView() with obsolete signature.');
var paramObj = {};
if (typeof params === 'number') {
paramObj.pageNumber = params; // pageNumber argument was found.
}
if (arguments[1] instanceof Array) {
paramObj.destArray = arguments[1]; // destArray argument was found.
}
params = paramObj;
}
//#endif
var pageNumber = params.pageNumber || 0;
var dest = params.destArray || null;
var allowNegativeOffset = params.allowNegativeOffset || false;
if (this.isInPresentationMode || !dest) {
this._setCurrentPageNumber(pageNumber, /* resetCurrentPageView */ true);
@ -549,6 +575,11 @@ var PDFViewer = (function pdfViewer() {
}
var pageView = this._pages[pageNumber - 1];
if (!pageView) {
console.error('PDFViewer_scrollPageIntoView: ' +
'Invalid "pageNumber" parameter.');
return;
}
var x = 0, y = 0;
var width = 0, height = 0, widthScale, heightScale;
var changeOrientation = (pageView.rotation % 180 === 0 ? false : true);
@ -627,14 +658,15 @@ var PDFViewer = (function pdfViewer() {
pageView.viewport.convertToViewportPoint(x + width, y + height)
];
var left = Math.min(boundingRect[0][0], boundingRect[1][0]);
// Some pdf generator will generate a large top value (e.g. 10000)
// for outline destination
// which exceeds the hight of the page
// Therefore we have to ensure top is not less 0
// otherwise viewer will scroll to previous page
// See PR 6903 and bug 874482 for more discussion
var top = Math.max(Math.min(boundingRect[0][1], boundingRect[1][1]), 0);
var top = Math.min(boundingRect[0][1], boundingRect[1][1]);
if (!allowNegativeOffset) {
// Some bad PDF generators will create destinations with e.g. top values
// that exceeds the page height. Ensure that offsets are not negative,
// to prevent a previous page from becoming visible (fixes bug 874482).
left = Math.max(left, 0);
top = Math.max(top, 0);
}
scrollIntoView(pageView.div, { left: left, top: top });
},