Move the page switching code into set currentPageNumber
in PDFViewer
instead of placing it in the pagechange
event handler
The reason that this code can be moved is that the `if (this.loading && page === 1)` check, in the `pagechange` event handler in viewer.js, is never satisfied since `this.loading` is not defined in that scope. This *could* be considered a regression from PR 5295, since prior to that `this.loading` was using the `PDFViewerApplication` scope (or `PDFView` as it were). However, I don't think that we need to fix that since we've been shipping this code in no less than *three* Firefox releases (uplifted in https://bugzilla.mozilla.org/show_bug.cgi?id=1084158), without breaking the world. An explanation of why the `pagechange` code works, despite `this.loading === undefined`, is that `set currentPageNumber` (in `PDFViewer`) returns early whenever `this.pdfDocument` isn't set. This check is, for all intents and purposes, functionally equivalent to checking `PDFViewerApplication.loading`. Hence we can move the page switching code into `PDFViewer`, and also remove `PDFViewerApplication.loading` since it's not used any more. (The `this.loading` property was added in PR 686, which was before the current viewer even existed.) *Note:* The changes in this patch should also be beneficial to the viewer `components`, since requiring every implementer to provide their own `pagechange` event handler just to get `PDFViewer.currentPageNumber` to actually work seems like an unnecessary complication.
This commit is contained in:
parent
18e1a14e65
commit
ffeba9c630
2 changed files with 8 additions and 15 deletions
|
@ -137,6 +137,12 @@ var PDFViewer = (function pdfViewer() {
|
|||
this._currentPageNumber = val;
|
||||
event.pageNumber = val;
|
||||
this.container.dispatchEvent(event);
|
||||
|
||||
// Check if the caller is `PDFViewer_update`, to avoid breaking scrolling.
|
||||
if (this.updateInProgress) {
|
||||
return;
|
||||
}
|
||||
this.scrollPageIntoView(val);
|
||||
},
|
||||
|
||||
/**
|
||||
|
@ -566,7 +572,7 @@ var PDFViewer = (function pdfViewer() {
|
|||
};
|
||||
},
|
||||
|
||||
update: function () {
|
||||
update: function PDFViewer_update() {
|
||||
var visible = this._getVisiblePages();
|
||||
var visiblePages = visible.views;
|
||||
if (visiblePages.length === 0) {
|
||||
|
@ -581,7 +587,7 @@ var PDFViewer = (function pdfViewer() {
|
|||
|
||||
this.renderingQueue.renderHighestPriority(visible);
|
||||
|
||||
var currentId = this.currentPageNumber;
|
||||
var currentId = this._currentPageNumber;
|
||||
var firstPage = visible.first;
|
||||
|
||||
for (var i = 0, ii = visiblePages.length, stillFullyVisible = false;
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue