Browse Source

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.
Jonas Jenwald 9 years ago
parent
commit
b7cb44af88
  1. 11
      web/app.js
  2. 1
      web/dom_events.js
  3. 15
      web/pdf_viewer.js

11
web/app.js

@ -1526,11 +1526,12 @@ function webViewerInitialized() {
}); });
appConfig.toolbar.pageNumber.addEventListener('change', function() { appConfig.toolbar.pageNumber.addEventListener('change', function() {
// Handle the user inputting a floating point number.
PDFViewerApplication.page = (this.value | 0); PDFViewerApplication.page = (this.value | 0);
if (this.value !== (this.value | 0).toString()) { // Ensure that the page number input displays the correct value, even if the
this.value = PDFViewerApplication.page; // 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({ PDFViewerApplication._updateUIToolbar({
pageNumber: page, pageNumber: page,
}); });
if (e.previousPageNumber !== page &&
PDFViewerApplication.pdfSidebar.isThumbnailViewVisible) { if (PDFViewerApplication.pdfSidebar.isThumbnailViewVisible) {
PDFViewerApplication.pdfThumbnailViewer.scrollThumbnailIntoView(page); PDFViewerApplication.pdfThumbnailViewer.scrollThumbnailIntoView(page);
} }

1
web/dom_events.js

@ -53,7 +53,6 @@
var event = document.createEvent('UIEvents'); var event = document.createEvent('UIEvents');
event.initUIEvent('pagechange', true, true, window, 0); event.initUIEvent('pagechange', true, true, window, 0);
event.pageNumber = e.pageNumber; event.pageNumber = e.pageNumber;
event.previousPageNumber = e.previousPageNumber;
e.source.container.dispatchEvent(event); e.source.container.dispatchEvent(event);
}); });
eventBus.on('pagesinit', function (e) { eventBus.on('pagesinit', function (e) {

15
web/pdf_viewer.js

@ -166,6 +166,9 @@ var PDFViewer = (function pdfViewer() {
* @param {number} val - The page number. * @param {number} val - The page number.
*/ */
set currentPageNumber(val) { set currentPageNumber(val) {
if ((val | 0) !== val) { // Ensure that `val` is an integer.
throw new Error('Invalid page number.');
}
if (!this.pdfDocument) { if (!this.pdfDocument) {
this._currentPageNumber = val; this._currentPageNumber = val;
return; return;
@ -185,22 +188,14 @@ var PDFViewer = (function pdfViewer() {
} }
return; return;
} }
var arg;
if (!(0 < val && val <= this.pagesCount)) { 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; return;
} }
arg = { var arg = {
source: this, source: this,
pageNumber: val, pageNumber: val,
previousPageNumber: this._currentPageNumber
}; };
this._currentPageNumber = val; this._currentPageNumber = val;
this.eventBus.dispatch('pagechanging', arg); this.eventBus.dispatch('pagechanging', arg);

Loading…
Cancel
Save