From 870a8f6c35790422f17560a54202bc1357604f94 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 24 Aug 2017 09:24:32 +0200 Subject: [PATCH] Remove the ability to pass a `scale` parameter in the (optional) `args` object parameter of `PDFViewerApplication.open(file, args)` Since the very early days of the viewer, it's been possible to pass in a `scale` when opening a PDF file. However, most of the time it was/is actually being ignored, which limits its usefulness considerably. In older versions of the viewer, if a document hash was present (i.e. `PDFViewerApplication.initialBookmark` being set) or if the document existed in the `ViewHistory`, the `scale` passed to `PDFViewerApplication.open` would thus always be ignored. In addition to the above, in the current viewer there's even more cases where the `scale` parameter will be ignored: if a (valid) browser history entry exists on document load, or if the `defaultZoomValue` preference is set to a non-default value. Hence the result is that in most situation, a `scale` passed to `PDFViewerApplication.open` will be completely ignored. A much better, not to mention supported, way of setting the initial scale is by using the `defaultZoomLevel` preference. In comparision, this also has the advantage of being used in situations where the `scale` would be ignored. All in all this leads to the current situation where we have code which is essentially dead, since no part of the viewer (by default) relies on it. To clean up this code, and to avoid having to pass (basically) unused parameters around, I'd thus like to remove the ability to pass a `scale` to `PDFViewerApplication.open`. --- web/app.js | 35 +++++++++++++++-------------------- web/pdf_sidebar.js | 2 +- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/web/app.js b/web/app.js index 0b4f0cd4a..95bbc88b6 100644 --- a/web/app.js +++ b/web/app.js @@ -16,8 +16,8 @@ import { animationStarted, DEFAULT_SCALE_VALUE, getPDFFileNameFromURL, MAX_SCALE, - MIN_SCALE, noContextMenuHandler, normalizeWheelEventDelta, - parseQueryString, ProgressBar, RendererType, UNKNOWN_SCALE + MIN_SCALE, noContextMenuHandler, normalizeWheelEventDelta, parseQueryString, + ProgressBar, RendererType } from './ui_utils'; import { build, createBlob, getDocument, getFilenameFromUrl, InvalidPDFException, @@ -649,7 +649,7 @@ let PDFViewerApplication = { }); } - let parameters = Object.create(null), scale; + let parameters = Object.create(null); if (typeof file === 'string') { // URL this.setTitleUsingUrl(file); parameters.url = file; @@ -666,15 +666,16 @@ let PDFViewerApplication = { if (args) { for (let prop in args) { + if ((typeof PDFJSDev === 'undefined' || !PDFJSDev.test('PDFJS_NEXT')) && + !PDFJS.pdfjsNext && prop === 'scale') { + console.error('Call of open() with obsolete "scale" argument, ' + + 'please use the "defaultZoomValue" preference instead.'); + continue; + } else if (prop === 'length') { + this.pdfDocumentProperties.setFileSize(args[prop]); + } parameters[prop] = args[prop]; } - - if (args.scale) { - scale = args.scale; - } - if (args.length) { - this.pdfDocumentProperties.setFileSize(args.length); - } } let loadingTask = getDocument(parameters); @@ -693,7 +694,7 @@ let PDFViewerApplication = { loadingTask.onUnsupportedFeature = this.fallback.bind(this); return loadingTask.promise.then((pdfDocument) => { - this.load(pdfDocument, scale); + this.load(pdfDocument); }, (exception) => { let message = exception && exception.message; let loadingErrorMessage; @@ -879,8 +880,7 @@ let PDFViewerApplication = { } }, - load(pdfDocument, scale) { - scale = scale || UNKNOWN_SCALE; + load(pdfDocument) { this.pdfDocument = pdfDocument; pdfDocument.getDownloadInfo().then(() => { @@ -977,7 +977,7 @@ let PDFViewerApplication = { sidebarView, }; }).then(({ hash, sidebarView, }) => { - this.setInitialView(hash, { sidebarView, scale, }); + this.setInitialView(hash, { sidebarView, }); initialParams.hash = hash; // Make all navigation keys work on document load, @@ -1135,9 +1135,7 @@ let PDFViewerApplication = { }); }, - setInitialView(storedHash, options = {}) { - let { scale = 0, sidebarView = SidebarView.NONE, } = options; - + setInitialView(storedHash, { sidebarView, } = {}) { this.isInitialViewSet = true; this.pdfSidebar.setInitialView(sidebarView); @@ -1150,9 +1148,6 @@ let PDFViewerApplication = { this.initialBookmark = null; } else if (storedHash) { this.pdfLinkService.setHash(storedHash); - } else if (scale) { - this.pdfViewer.currentScaleValue = scale; - this.page = 1; } // Ensure that the correct page number is displayed in the UI, diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js index caabedd57..89cfcf58c 100644 --- a/web/pdf_sidebar.js +++ b/web/pdf_sidebar.js @@ -126,7 +126,7 @@ class PDFSidebar { * @param {number} view - The sidebar view that should become visible, * must be one of the values in {SidebarView}. */ - setInitialView(view) { + setInitialView(view = SidebarView.NONE) { if (this.isInitialViewSet) { return; }