From e522b2d87e8eb3d4eb8a391a098a0f6f157e8c12 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jun 2018 13:02:23 +0200 Subject: [PATCH 1/8] Remove the unused `PDFViewerApplication.documentFingerprint` property This property isn't accessed anywhere in the `web/app.js` file, and is also not being reset in `PDFViewerApplication.close`. Hence it seems that it can simply be removed, especially since the fingerprint is already synchronously available through `PDFViewerApplication.pdfDocument.fingerprint` (provided that a document is loaded). --- web/app.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/web/app.js b/web/app.js index e4f01cf15..6f704d658 100644 --- a/web/app.js +++ b/web/app.js @@ -973,8 +973,7 @@ let PDFViewerApplication = { this.toolbar.setPagesCount(pdfDocument.numPages, false); this.secondaryToolbar.setPagesCount(pdfDocument.numPages); - let id = this.documentFingerprint = pdfDocument.fingerprint; - let store = this.store = new ViewHistory(id); + const store = this.store = new ViewHistory(pdfDocument.fingerprint); let baseDocumentUrl; if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { @@ -1003,7 +1002,7 @@ let PDFViewerApplication = { // The browsing history is only enabled when the viewer is standalone, // i.e. not when it is embedded in a web page. let resetHistory = !AppOptions.get('showPreviousViewOnLoad'); - this.pdfHistory.initialize(id, resetHistory); + this.pdfHistory.initialize(pdfDocument.fingerprint, resetHistory); if (this.pdfHistory.initialBookmark) { this.initialBookmark = this.pdfHistory.initialBookmark; From baf9c98bc720707e707d96dcac66a3e1b6f434cf Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jun 2018 13:07:37 +0200 Subject: [PATCH 2/8] Add `scrollModeOnLoad`/`spreadModeOnLoad` default values to `AppOptions` For some reason, these weren't added to `AppOptions` despite actually being set and read from `web/app.js`. Not adding them creates inconsistencies, since all other options *are* present in `web/app_options.js`. --- web/app_options.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/web/app_options.js b/web/app_options.js index a6023264d..83b7d3d47 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -116,6 +116,16 @@ const defaultOptions = { value: 0, kind: OptionKind.VIEWER, }, + scrollModeOnLoad: { + /** @type {number} */ + value: 0, + kind: OptionKind.VIEWER, + }, + spreadModeOnLoad: { + /** @type {number} */ + value: 0, + kind: OptionKind.VIEWER, + }, textLayerMode: { /** @type {number} */ value: 1, From a9a93bd92398e2082bd018631f2ed0289f9cd98d Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jun 2018 13:16:00 +0200 Subject: [PATCH 3/8] Allow the `scrollModeOnLoad`/`spreadModeOnLoad` Preferences to override ViewHistory settings Note how the other "...OnLoad" preferences will allow a *non-default* value to always override a history entry. To improve overall consistency for the viewer options, and to reduce possibly confusing behaviour, this patch changes the `scrollModeOnLoad`/`spreadModeOnLoad` preferences to behave as all the other ones. --- web/app.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/web/app.js b/web/app.js index 6f704d658..c924db3c5 100644 --- a/web/app.js +++ b/web/app.js @@ -1043,12 +1043,8 @@ let PDFViewerApplication = { rotation = parseInt(values.rotation, 10); sidebarView = sidebarView || (values.sidebarView | 0); - if (values.scrollMode !== null) { - scrollMode = values.scrollMode; - } - if (values.spreadMode !== null) { - spreadMode = values.spreadMode; - } + scrollMode = scrollMode || (values.scrollMode | 0); + spreadMode = spreadMode || (values.spreadMode | 0); } if (pageMode && !AppOptions.get('disablePageMode')) { // Always let the user preference/history take precedence. @@ -1245,23 +1241,26 @@ let PDFViewerApplication = { }); }, - setInitialView(storedHash, values = {}) { - let { rotation, sidebarView, scrollMode, spreadMode, } = values; + setInitialView(storedHash, { rotation, sidebarView, + scrollMode, spreadMode, } = {}) { let setRotation = (angle) => { if (isValidRotation(angle)) { this.pdfViewer.pagesRotation = angle; } }; + let setViewerModes = (scroll, spread) => { + if (Number.isInteger(scroll)) { + this.pdfViewer.setScrollMode(scroll); + } + if (Number.isInteger(spread)) { + this.pdfViewer.setSpreadMode(spread); + } + }; // Putting these before isInitialViewSet = true prevents these values from // being stored in the document history (and overriding any future changes // made to the corresponding global preferences), just this once. - if (Number.isInteger(scrollMode)) { - this.pdfViewer.setScrollMode(scrollMode); - } - if (Number.isInteger(spreadMode)) { - this.pdfViewer.setSpreadMode(spreadMode); - } + setViewerModes(scrollMode, spreadMode); this.isInitialViewSet = true; this.pdfSidebar.setInitialView(sidebarView); From 9515f579c6ff0b9147c852ebb6fd4a6761d907cc Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jun 2018 14:14:11 +0200 Subject: [PATCH 4/8] Change the `scrollMode`/`spreadMode` to "private" properties on `BaseViewer` instances --- web/base_viewer.js | 12 ++++++------ web/pdf_viewer.js | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 79c65c9d4..4bccae26e 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -162,8 +162,8 @@ class BaseViewer { this.useOnlyCssZoom = options.useOnlyCssZoom || false; this.maxCanvasPixels = options.maxCanvasPixels; this.l10n = options.l10n || NullL10n; - this.scrollMode = options.scrollMode || ScrollMode.VERTICAL; - this.spreadMode = options.spreadMode || SpreadMode.NONE; + this._scrollMode = options.scrollMode || ScrollMode.VERTICAL; + this._spreadMode = options.spreadMode || SpreadMode.NONE; this.defaultRenderingQueue = !options.renderingQueue; if (this.defaultRenderingQueue) { @@ -181,7 +181,7 @@ class BaseViewer { if (this.removePageBorders) { this.viewer.classList.add('removePageBorders'); } - if (this.scrollMode !== ScrollMode.VERTICAL) { + if (this._scrollMode !== ScrollMode.VERTICAL) { this._updateScrollModeClasses(); } } @@ -441,7 +441,7 @@ class BaseViewer { bindOnAfterAndBeforeDraw(pageView); this._pages.push(pageView); } - if (this.spreadMode !== SpreadMode.NONE) { + if (this._spreadMode !== SpreadMode.NONE) { this._regroupSpreads(); } @@ -1027,7 +1027,7 @@ class BaseViewer { if (!Number.isInteger(mode) || !Object.values(ScrollMode).includes(mode)) { throw new Error(`Invalid scroll mode: ${mode}`); } - this.scrollMode = mode; + this._scrollMode = mode; } _updateScrollModeClasses() { @@ -1038,7 +1038,7 @@ class BaseViewer { if (!Number.isInteger(mode) || !Object.values(SpreadMode).includes(mode)) { throw new Error(`Invalid spread mode: ${mode}`); } - this.spreadMode = mode; + this._spreadMode = mode; } _regroupSpreads() { diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 0a7db67ce..230895b20 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -27,7 +27,7 @@ class PDFViewer extends BaseViewer { const left = pageDiv.offsetLeft + pageDiv.clientLeft; const right = left + pageDiv.clientWidth; const { scrollLeft, clientWidth, } = this.container; - if (this.scrollMode === ScrollMode.HORIZONTAL || + if (this._scrollMode === ScrollMode.HORIZONTAL || left < scrollLeft || right > scrollLeft + clientWidth) { pageSpot = { left: 0, top: 0, }; } @@ -38,7 +38,7 @@ class PDFViewer extends BaseViewer { _getVisiblePages() { if (!this.isInPresentationMode) { return getVisibleElements(this.container, this._pages, true, - this.scrollMode === ScrollMode.HORIZONTAL); + this._scrollMode === ScrollMode.HORIZONTAL); } // The algorithm in getVisibleElements doesn't work in all browsers and // configurations when presentation mode is active. @@ -91,11 +91,11 @@ class PDFViewer extends BaseViewer { // 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); + false : this._scrollMode === ScrollMode.HORIZONTAL); } setScrollMode(mode) { - if (mode === this.scrollMode) { + if (mode === this._scrollMode) { return; } super.setScrollMode(mode); @@ -118,7 +118,7 @@ class PDFViewer extends BaseViewer { } _updateScrollModeClasses() { - const { scrollMode, viewer, } = this; + const scrollMode = this._scrollMode, viewer = this.viewer; if (scrollMode === ScrollMode.HORIZONTAL) { viewer.classList.add('scrollHorizontal'); @@ -133,7 +133,7 @@ class PDFViewer extends BaseViewer { } setSpreadMode(mode) { - if (mode === this.spreadMode) { + if (mode === this._spreadMode) { return; } super.setSpreadMode(mode); @@ -150,12 +150,12 @@ class PDFViewer extends BaseViewer { // Temporarily remove all the pages from the DOM. viewer.textContent = ''; - if (this.spreadMode === SpreadMode.NONE) { + if (this._spreadMode === SpreadMode.NONE) { for (let i = 0, iMax = pages.length; i < iMax; ++i) { viewer.appendChild(pages[i].div); } } else { - const parity = this.spreadMode - 1; + const parity = this._spreadMode - 1; let spread = null; for (let i = 0, iMax = pages.length; i < iMax; ++i) { if (spread === null) { From a7ac27e385baeeae70b7db62efc6cf5474622ac8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jun 2018 15:07:42 +0200 Subject: [PATCH 5/8] Replace `setScrollMode`/`setSpreadMode` methods with getters/setters Since all the other viewer methods use the getter/setter pattern, e.g. for setting page/scale/rotation, the way that the Scroll/Spread modes are set thus stands out. For consistency, this really ought to use the same pattern as the rest of the `BaseViewer`. (To avoid breaking third-party implementations, the old methods are kept around as aliases.) --- web/app.js | 8 +++--- web/base_viewer.js | 61 ++++++++++++++++++++++++++++++++++++++++++++-- web/pdf_viewer.js | 33 ------------------------- 3 files changed, 63 insertions(+), 39 deletions(-) diff --git a/web/app.js b/web/app.js index c924db3c5..fe6916cb1 100644 --- a/web/app.js +++ b/web/app.js @@ -1250,10 +1250,10 @@ let PDFViewerApplication = { }; let setViewerModes = (scroll, spread) => { if (Number.isInteger(scroll)) { - this.pdfViewer.setScrollMode(scroll); + this.pdfViewer.scrollMode = scroll; } if (Number.isInteger(spread)) { - this.pdfViewer.setSpreadMode(spread); + this.pdfViewer.spreadMode = spread; } }; @@ -2019,10 +2019,10 @@ function webViewerRotateCcw() { PDFViewerApplication.rotatePages(-90); } function webViewerSwitchScrollMode(evt) { - PDFViewerApplication.pdfViewer.setScrollMode(evt.mode); + PDFViewerApplication.pdfViewer.scrollMode = evt.mode; } function webViewerSwitchSpreadMode(evt) { - PDFViewerApplication.pdfViewer.setSpreadMode(evt.mode); + PDFViewerApplication.pdfViewer.spreadMode = evt.mode; } function webViewerDocumentProperties() { PDFViewerApplication.pdfDocumentProperties.open(); diff --git a/web/base_viewer.js b/web/base_viewer.js index 4bccae26e..d5d7b3d2b 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1023,22 +1023,79 @@ class BaseViewer { }); } - setScrollMode(mode) { + /** + * @return {number} One of the values in {ScrollMode}. + */ + get scrollMode() { + return this._scrollMode; + } + + /** + * @param {number} mode - The direction in which the document pages should be + * laid out within the scrolling container. + * The constants from {ScrollMode} should be used. + */ + set scrollMode(mode) { + if (this._scrollMode === mode) { + return; // The Scroll mode didn't change. + } if (!Number.isInteger(mode) || !Object.values(ScrollMode).includes(mode)) { throw new Error(`Invalid scroll mode: ${mode}`); } this._scrollMode = mode; + this.eventBus.dispatch('scrollmodechanged', { source: this, mode, }); + + this._updateScrollModeClasses(); + + if (!this.pdfDocument) { + return; + } + const pageNumber = this._currentPageNumber; + // Non-numeric scale values can be sensitive to the scroll orientation. + // Call this before re-scrolling to the current page, to ensure that any + // changes in scale don't move the current page. + if (isNaN(this._currentScaleValue)) { + this._setScale(this._currentScaleValue, true); + } + this.scrollPageIntoView({ pageNumber, }); + this.update(); + } + + setScrollMode(mode) { + this.scrollMode = mode; } _updateScrollModeClasses() { // No-op in the base class. } - setSpreadMode(mode) { + /** + * @return {number} One of the values in {SpreadMode}. + */ + get spreadMode() { + return this._spreadMode; + } + + /** + * @param {number} mode - Group the pages in spreads, starting with odd- or + * even-number pages (unless `SpreadMode.NONE` is used). + * The constants from {SpreadMode} should be used. + */ + set spreadMode(mode) { + if (this._spreadMode === mode) { + return; // The Spread mode didn't change. + } if (!Number.isInteger(mode) || !Object.values(SpreadMode).includes(mode)) { throw new Error(`Invalid spread mode: ${mode}`); } this._spreadMode = mode; + this.eventBus.dispatch('spreadmodechanged', { source: this, mode, }); + + this._regroupSpreads(); + } + + setSpreadMode(mode) { + this.spreadMode = mode; } _regroupSpreads() { diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 230895b20..b49fa16f5 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -94,29 +94,6 @@ class PDFViewer extends BaseViewer { false : this._scrollMode === ScrollMode.HORIZONTAL); } - setScrollMode(mode) { - if (mode === this._scrollMode) { - return; - } - super.setScrollMode(mode); - - this.eventBus.dispatch('scrollmodechanged', { mode, }); - this._updateScrollModeClasses(); - - if (!this.pdfDocument) { - return; - } - const pageNumber = this._currentPageNumber; - // Non-numeric scale modes can be sensitive to the scroll orientation. - // Call this before re-scrolling to the current page, to ensure that any - // changes in scale don't move the current page. - if (isNaN(this._currentScaleValue)) { - this._setScale(this._currentScaleValue, true); - } - this.scrollPageIntoView({ pageNumber, }); - this.update(); - } - _updateScrollModeClasses() { const scrollMode = this._scrollMode, viewer = this.viewer; @@ -132,16 +109,6 @@ class PDFViewer extends BaseViewer { } } - setSpreadMode(mode) { - if (mode === this._spreadMode) { - return; - } - super.setSpreadMode(mode); - - this.eventBus.dispatch('spreadmodechanged', { mode, }); - this._regroupSpreads(); - } - _regroupSpreads() { if (!this.pdfDocument) { return; From 8b85ae4181ac7d46f7a741cb2d749c95e68b939b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jun 2018 15:28:38 +0200 Subject: [PATCH 6/8] Re-factor updating of Scroll/Spread modes, and place all the code in `BaseViewer` with overrides (as necessary) in the extending classes This structure probably makes somewhat more sense, given that `PDFSinglePageViewer` is somewhat of a special case. --- web/base_viewer.js | 71 +++++++++++++++++++++++++++-------- web/pdf_single_page_viewer.js | 4 ++ web/pdf_viewer.js | 48 +---------------------- 3 files changed, 61 insertions(+), 62 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index d5d7b3d2b..2b9901137 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -182,7 +182,7 @@ class BaseViewer { this.viewer.classList.add('removePageBorders'); } if (this._scrollMode !== ScrollMode.VERTICAL) { - this._updateScrollModeClasses(); + this._updateScrollMode(); } } @@ -442,7 +442,7 @@ class BaseViewer { this._pages.push(pageView); } if (this._spreadMode !== SpreadMode.NONE) { - this._regroupSpreads(); + this._updateSpreadMode(); } // Fetch all the pages since the viewport is needed before printing @@ -1045,16 +1045,30 @@ class BaseViewer { this._scrollMode = mode; this.eventBus.dispatch('scrollmodechanged', { source: this, mode, }); - this._updateScrollModeClasses(); + this._updateScrollMode(/* pageNumber = */ this._currentPageNumber); + } - if (!this.pdfDocument) { + _updateScrollMode(pageNumber = null) { + const scrollMode = this._scrollMode, viewer = this.viewer; + + if (scrollMode === ScrollMode.HORIZONTAL) { + viewer.classList.add('scrollHorizontal'); + } else { + viewer.classList.remove('scrollHorizontal'); + } + if (scrollMode === ScrollMode.WRAPPED) { + viewer.classList.add('scrollWrapped'); + } else { + viewer.classList.remove('scrollWrapped'); + } + + if (!this.pdfDocument || !pageNumber) { return; } - const pageNumber = this._currentPageNumber; // Non-numeric scale values can be sensitive to the scroll orientation. // Call this before re-scrolling to the current page, to ensure that any // changes in scale don't move the current page. - if (isNaN(this._currentScaleValue)) { + if (this._currentScaleValue && isNaN(this._currentScaleValue)) { this._setScale(this._currentScaleValue, true); } this.scrollPageIntoView({ pageNumber, }); @@ -1065,10 +1079,6 @@ class BaseViewer { this.scrollMode = mode; } - _updateScrollModeClasses() { - // No-op in the base class. - } - /** * @return {number} One of the values in {SpreadMode}. */ @@ -1091,16 +1101,47 @@ class BaseViewer { this._spreadMode = mode; this.eventBus.dispatch('spreadmodechanged', { source: this, mode, }); - this._regroupSpreads(); + this._updateSpreadMode(/* pageNumber = */ this._currentPageNumber); + } + + _updateSpreadMode(pageNumber = null) { + if (!this.pdfDocument) { + return; + } + const viewer = this.viewer, pages = this._pages; + // Temporarily remove all the pages from the DOM. + viewer.textContent = ''; + + if (this._spreadMode === SpreadMode.NONE) { + for (let i = 0, iMax = pages.length; i < iMax; ++i) { + viewer.appendChild(pages[i].div); + } + } else { + const parity = this._spreadMode - 1; + let spread = null; + for (let i = 0, iMax = pages.length; i < iMax; ++i) { + if (spread === null) { + spread = document.createElement('div'); + spread.className = 'spread'; + viewer.appendChild(spread); + } else if (i % 2 === parity) { + spread = spread.cloneNode(false); + viewer.appendChild(spread); + } + spread.appendChild(pages[i].div); + } + } + + if (!pageNumber) { + return; + } + this.scrollPageIntoView({ pageNumber, }); + this.update(); } setSpreadMode(mode) { this.spreadMode = mode; } - - _regroupSpreads() { - // No-op in the base class. - } } export { diff --git a/web/pdf_single_page_viewer.js b/web/pdf_single_page_viewer.js index 4a446f457..575b485e8 100644 --- a/web/pdf_single_page_viewer.js +++ b/web/pdf_single_page_viewer.js @@ -147,6 +147,10 @@ class PDFSinglePageViewer extends BaseViewer { // The Scroll/Spread modes are never used in `PDFSinglePageViewer`. return shadow(this, '_isScrollModeHorizontal', false); } + + _updateScrollMode() { } + + _updateSpreadMode() { } } export { diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index b49fa16f5..659c8db76 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -13,7 +13,7 @@ * limitations under the License. */ -import { BaseViewer, ScrollMode, SpreadMode } from './base_viewer'; +import { BaseViewer, ScrollMode } from './base_viewer'; import { getVisibleElements, scrollIntoView } from './ui_utils'; import { shadow } from 'pdfjs-lib'; @@ -93,52 +93,6 @@ class PDFViewer extends BaseViewer { return (this.isInPresentationMode ? false : this._scrollMode === ScrollMode.HORIZONTAL); } - - _updateScrollModeClasses() { - const scrollMode = this._scrollMode, viewer = this.viewer; - - if (scrollMode === ScrollMode.HORIZONTAL) { - viewer.classList.add('scrollHorizontal'); - } else { - viewer.classList.remove('scrollHorizontal'); - } - if (scrollMode === ScrollMode.WRAPPED) { - viewer.classList.add('scrollWrapped'); - } else { - viewer.classList.remove('scrollWrapped'); - } - } - - _regroupSpreads() { - if (!this.pdfDocument) { - return; - } - const viewer = this.viewer, pages = this._pages; - // Temporarily remove all the pages from the DOM. - viewer.textContent = ''; - - if (this._spreadMode === SpreadMode.NONE) { - for (let i = 0, iMax = pages.length; i < iMax; ++i) { - viewer.appendChild(pages[i].div); - } - } else { - const parity = this._spreadMode - 1; - let spread = null; - for (let i = 0, iMax = pages.length; i < iMax; ++i) { - if (spread === null) { - spread = document.createElement('div'); - spread.className = 'spread'; - viewer.appendChild(spread); - } else if (i % 2 === parity) { - spread = spread.cloneNode(false); - viewer.appendChild(spread); - } - spread.appendChild(pages[i].div); - } - } - this.scrollPageIntoView({ pageNumber: this._currentPageNumber, }); - this.update(); - } } export { From bb193dc501c1cf0b9c1d177e164e3cc9942d54ee Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jun 2018 15:48:45 +0200 Subject: [PATCH 7/8] For consistency with other viewer state, remove and deprecate setting Scroll/Spread modes in the `BaseViewer` constructor Since other viewer state, such as the current page/scale/rotation[1], are not available as `BaseViewer` constructor options, this makes the Scroll/Spread modes stand out quite a bit. Hence it probably makes sense to remove/deprecate this, to avoid inconsistent and possibly confusing state in this code. --- [1] These properties are *purposely* not available in the constructor, since attempting to set them before a document is loaded has number of issues; please refer to https://github.com/mozilla/pdf.js/pull/8539#issuecomment-309706629 for additional details. --- web/base_viewer.js | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 2b9901137..c904a4481 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -73,14 +73,6 @@ const SpreadMode = { * size in total pixels, i.e. width * height. Use -1 for no limit. * The default value is 4096 * 4096 (16 mega-pixels). * @property {IL10n} l10n - Localization service. - * @property {number} scrollMode - (optional) The direction in which the - * document pages should be laid out within the scrolling container. The - * constants from {ScrollMode} should be used. The default value is - * `ScrollMode.VERTICAL`. - * @property {number} spreadMode - (optional) If not `SpreadMode.NONE`, groups - * pages into spreads, starting with odd- or even-numbered pages. The - * constants from {SpreadMode} should be used. The default value is - * `SpreadMode.NONE`. */ function PDFPageViewBuffer(size) { @@ -162,8 +154,6 @@ class BaseViewer { this.useOnlyCssZoom = options.useOnlyCssZoom || false; this.maxCanvasPixels = options.maxCanvasPixels; this.l10n = options.l10n || NullL10n; - this._scrollMode = options.scrollMode || ScrollMode.VERTICAL; - this._spreadMode = options.spreadMode || SpreadMode.NONE; this.defaultRenderingQueue = !options.renderingQueue; if (this.defaultRenderingQueue) { @@ -181,8 +171,17 @@ class BaseViewer { if (this.removePageBorders) { this.viewer.classList.add('removePageBorders'); } - if (this._scrollMode !== ScrollMode.VERTICAL) { - this._updateScrollMode(); + + if ((typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) && + ('scrollMode' in options || 'spreadMode' in options)) { + console.error(`The ${this._name} constructor options ` + + '`scrollMode`/`spreadMode` are deprecated, use the setters instead.'); + if (options.scrollMode !== undefined) { + this.scrollMode = options.scrollMode; + } + if (options.spreadMode !== undefined) { + this.spreadMode = options.spreadMode; + } } } @@ -524,9 +523,13 @@ class BaseViewer { this._pagesRotation = 0; this._pagesRequests = []; this._pageViewsReady = false; + this._scrollMode = ScrollMode.VERTICAL; + this._spreadMode = SpreadMode.NONE; - // Remove the pages from the DOM. + // Remove the pages from the DOM... this.viewer.textContent = ''; + // ... and reset the Scroll mode CSS class(es) afterwards. + this._updateScrollMode(); } _scrollUpdate() { @@ -1076,7 +1079,11 @@ class BaseViewer { } setScrollMode(mode) { - this.scrollMode = mode; + if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { + console.error(`${this._name}.setScrollMode() is deprecated, ` + + `use the ${this._name}.scrollMode setter instead.`); + this.scrollMode = mode; + } } /** @@ -1140,7 +1147,11 @@ class BaseViewer { } setSpreadMode(mode) { - this.spreadMode = mode; + if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { + console.error(`${this._name}.setSpreadMode() is deprecated, ` + + `use the ${this._name}.spreadMode setter instead.`); + this.spreadMode = mode; + } } } From ff26d419dda98ccc8449c0269573d287c94dbeee Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 30 Jun 2018 14:54:33 +0200 Subject: [PATCH 8/8] Ensure that Scroll/Spread mode buttons are correctly reset, when the document is closed Since the Scroll/Spread modes are now document specific, as all other properties such as page/scale/rotation, ensure that the toolbar is always correctly reset. --- web/secondary_toolbar.js | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/web/secondary_toolbar.js b/web/secondary_toolbar.js index 233f5f6fa..ac46becde 100644 --- a/web/secondary_toolbar.js +++ b/web/secondary_toolbar.js @@ -140,6 +140,10 @@ class SecondaryToolbar { this.pageNumber = 0; this.pagesCount = 0; this._updateUIState(); + + // Reset the Scroll/Spread buttons too, since they're document specific. + this.eventBus.dispatch('resetscrollmode', { source: this, }); + this.eventBus.dispatch('resetspreadmode', { source: this, }); } _updateUIState() { @@ -189,7 +193,7 @@ class SecondaryToolbar { } _bindScrollModeListener(buttons) { - this.eventBus.on('scrollmodechanged', function(evt) { + function scrollModeChanged(evt) { buttons.scrollVerticalButton.classList.remove('toggled'); buttons.scrollHorizontalButton.classList.remove('toggled'); buttons.scrollWrappedButton.classList.remove('toggled'); @@ -205,11 +209,18 @@ class SecondaryToolbar { buttons.scrollWrappedButton.classList.add('toggled'); break; } + } + this.eventBus.on('scrollmodechanged', scrollModeChanged); + + this.eventBus.on('resetscrollmode', (evt) => { + if (evt.source === this) { + scrollModeChanged({ mode: ScrollMode.VERTICAL, }); + } }); } _bindSpreadModeListener(buttons) { - this.eventBus.on('spreadmodechanged', function(evt) { + function spreadModeChanged(evt) { buttons.spreadNoneButton.classList.remove('toggled'); buttons.spreadOddButton.classList.remove('toggled'); buttons.spreadEvenButton.classList.remove('toggled'); @@ -225,6 +236,13 @@ class SecondaryToolbar { buttons.spreadEvenButton.classList.add('toggled'); break; } + } + this.eventBus.on('spreadmodechanged', spreadModeChanged); + + this.eventBus.on('resetspreadmode', (evt) => { + if (evt.source === this) { + spreadModeChanged({ mode: SpreadMode.NONE, }); + } }); }