From b7cb44af88f0bc2da89b893d5db6b1fe5bb8281f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 22 Jul 2016 15:32:14 +0200 Subject: [PATCH] Remove the `previousPageNumber` parameter from the `pagechanging`/pagechange` events, and stop dispatching the events if the input is out of bounds This patch attempts to cleanup a couple of things: - Remove the `previousPageNumber` paramater. Prior to PR 7289, when the events were dispatched even when the active page didn't change, it made sense to be able to detect that in an event listener. However, now that's no longer the case, and furthermore other similar events (e.g. `scalechanging`/`scalechange`) don't include information about the previous state. - Don't dispatch the events when the value passed to `set currentPageNumber` is out of bounds. Given that the active page doesn't change in this case, again similar to PR 7289, I don't think that the events should actually be dispatched in this case. - Ensure that the value passed to `set currentPageNumber` is actually an integer, to avoid any issues (note how e.g. `set currentScale` has similar validation code). Given that these changes could possibly affect the PDF.js `mochitest` integration tests in mozilla-central, in particular https://dxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/test/browser_pdfjs_navigation.js, I ran the tests locally with this patch applied to ensure that they still pass. --- web/app.js | 11 ++++++----- web/dom_events.js | 1 - web/pdf_viewer.js | 17 ++++++----------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/web/app.js b/web/app.js index 6446aa986..23179622c 100644 --- a/web/app.js +++ b/web/app.js @@ -1526,11 +1526,12 @@ function webViewerInitialized() { }); appConfig.toolbar.pageNumber.addEventListener('change', function() { - // Handle the user inputting a floating point number. PDFViewerApplication.page = (this.value | 0); - if (this.value !== (this.value | 0).toString()) { - this.value = PDFViewerApplication.page; + // Ensure that the page number input displays the correct value, even if the + // value entered by the user was invalid (e.g. a floating point number). + if (this.value !== PDFViewerApplication.page.toString()) { + PDFViewerApplication._updateUIToolbar({}); } }); @@ -1971,8 +1972,8 @@ function webViewerPageChanging(e) { PDFViewerApplication._updateUIToolbar({ pageNumber: page, }); - if (e.previousPageNumber !== page && - PDFViewerApplication.pdfSidebar.isThumbnailViewVisible) { + + if (PDFViewerApplication.pdfSidebar.isThumbnailViewVisible) { PDFViewerApplication.pdfThumbnailViewer.scrollThumbnailIntoView(page); } diff --git a/web/dom_events.js b/web/dom_events.js index 80893f52a..ae580eb70 100644 --- a/web/dom_events.js +++ b/web/dom_events.js @@ -53,7 +53,6 @@ var event = document.createEvent('UIEvents'); event.initUIEvent('pagechange', true, true, window, 0); event.pageNumber = e.pageNumber; - event.previousPageNumber = e.previousPageNumber; e.source.container.dispatchEvent(event); }); eventBus.on('pagesinit', function (e) { diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 0207c6f78..50c69621f 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -166,6 +166,9 @@ var PDFViewer = (function pdfViewer() { * @param {number} val - The page number. */ set currentPageNumber(val) { + if ((val | 0) !== val) { // Ensure that `val` is an integer. + throw new Error('Invalid page number.'); + } if (!this.pdfDocument) { this._currentPageNumber = val; return; @@ -185,22 +188,14 @@ var PDFViewer = (function pdfViewer() { } return; } - var arg; + if (!(0 < val && val <= this.pagesCount)) { - arg = { - source: this, - pageNumber: this._currentPageNumber, - previousPageNumber: val - }; - this.eventBus.dispatch('pagechanging', arg); - this.eventBus.dispatch('pagechange', arg); return; } - arg = { + var arg = { source: this, pageNumber: val, - previousPageNumber: this._currentPageNumber }; this._currentPageNumber = val; this.eventBus.dispatch('pagechanging', arg); @@ -223,7 +218,7 @@ var PDFViewer = (function pdfViewer() { * @param {number} val - Scale of the pages in percents. */ set currentScale(val) { - if (isNaN(val)) { + if (isNaN(val)) { throw new Error('Invalid numeric scale'); } if (!this.pdfDocument) {