Skip to content

Commit

Permalink
[Regression] Ensure that pre-rendering of the next/previous page work…
Browse files Browse the repository at this point in the history
…s correctly in Presentation Mode, when horizontal scrolling was enabled

Note how in `BaseViewer.forceRendering` the Scroll mode is used to determine how pre-rendering will work. Currently this is broken in Presentation Mode, if horizontal scrolling was enabled prior to entering fullscreen.

Furthermore, there's a few additional cases where the `this.scrollMode === ScrollMode.HORIZONTAL` check is pointless either in Presentation Mode or when a `PDFSinglePageViewer` instance is used.
  • Loading branch information
Snuffleupagus committed Jun 22, 2018
1 parent 63f63dd commit 550538d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 8 deletions.
18 changes: 11 additions & 7 deletions web/base_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -597,11 +597,11 @@ class BaseViewer {
if (!currentPage) {
return;
}
let hPadding = (this.isInPresentationMode || this.removePageBorders) ?
0 : SCROLLBAR_PADDING;
let vPadding = (this.isInPresentationMode || this.removePageBorders) ?
0 : VERTICAL_PADDING;
if (this.scrollMode === ScrollMode.HORIZONTAL) {
const noPadding = (this.isInPresentationMode || this.removePageBorders);
let hPadding = noPadding ? 0 : SCROLLBAR_PADDING;
let vPadding = noPadding ? 0 : VERTICAL_PADDING;

if (!noPadding && this._isScrollModeHorizontal) {
const temp = hPadding;
hPadding = vPadding;
vPadding = temp;
Expand Down Expand Up @@ -834,6 +834,10 @@ class BaseViewer {
this.container.focus();
}

get _isScrollModeHorizontal() {
throw new Error('Not implemented: _isScrollModeHorizontal');
}

get isInPresentationMode() {
return this.presentationModeState === PresentationModeState.FULLSCREEN;
}
Expand Down Expand Up @@ -906,8 +910,8 @@ class BaseViewer {

forceRendering(currentlyVisiblePages) {
let visiblePages = currentlyVisiblePages || this._getVisiblePages();
let scrollAhead = this.scrollMode === ScrollMode.HORIZONTAL ?
this.scroll.right : this.scroll.down;
let scrollAhead = (this._isScrollModeHorizontal ?
this.scroll.right : this.scroll.down);
let pageView = this.renderingQueue.getHighestPriority(visiblePages,
this._pages,
scrollAhead);
Expand Down
5 changes: 5 additions & 0 deletions web/pdf_single_page_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ class PDFSinglePageViewer extends BaseViewer {
location: this._location,
});
}

get _isScrollModeHorizontal() {
// The Scroll/Spread modes are never used in `PDFSinglePageViewer`.
return shadow(this, '_isScrollModeHorizontal', false);
}
}

export {
Expand Down
9 changes: 8 additions & 1 deletion web/pdf_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class PDFViewer extends BaseViewer {
}

_scrollIntoView({ pageDiv, pageSpot = null, }) {
if (!pageSpot) {
if (!pageSpot && !this.isInPresentationMode) {
const left = pageDiv.offsetLeft + pageDiv.clientLeft;
const right = left + pageDiv.clientWidth;
const { scrollLeft, clientWidth, } = this.container;
Expand Down Expand Up @@ -87,6 +87,13 @@ class PDFViewer extends BaseViewer {
});
}

get _isScrollModeHorizontal() {
// Used to ensure that pre-rendering of the next/previous page works
// correctly, since Scroll/Spread modes are ignored in Presentation Mode.
return (this.isInPresentationMode ?
false : this.scrollMode === ScrollMode.HORIZONTAL);
}

setScrollMode(mode) {
if (mode === this.scrollMode) {
return;
Expand Down

0 comments on commit 550538d

Please sign in to comment.