From f0a28b3c0d7b72f16cf31d1592b7dbbdc5c23578 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 6 Apr 2019 11:04:44 +0200 Subject: [PATCH] [Firefox] Ensure that loading progress is reported, and the loadingBar updated, when `disableRange=true` is set With PR 10675 having fixed the completely broken `disableRange=true` setting in the Firefox version of PDF.js, I couldn't help but noticing that loading progress is never reported properly in that case. Currently loading progress is only reported for the `rangeProgress` chrome-event, which obviously isn't dispatched with `disableRange=true` set. However, the `progressiveRead` chrome-event includes loading progress as well, but this information isn't being used in any way. Furthermore, the `PDFDataRangeTransport.onDataProgress` method wasn't able to handle "complete" loading information, and neither was `PDFDataTransportStream._onProgress` since that method would only ever attempt to report it through a RangeReader (which won't exist when `disableRange=true` is set). --- src/display/api.js | 5 +++-- src/display/transport_stream.js | 11 ++++++++--- web/firefoxcom.js | 4 ++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index f842490474e17..e77cc6059a78c 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -515,6 +515,7 @@ const PDFDocumentLoadingTask = (function PDFDocumentLoadingTaskClosure() { * Abstract class to support range requests file loading. * @param {number} length * @param {Uint8Array} initialData + * @param {boolean} progressiveDone */ class PDFDataRangeTransport { constructor(length, initialData, progressiveDone = false) { @@ -551,10 +552,10 @@ class PDFDataRangeTransport { } } - onDataProgress(loaded) { + onDataProgress(loaded, total) { this._readyCapability.promise.then(() => { for (const listener of this._progressListeners) { - listener(loaded); + listener(loaded, total); } }); } diff --git a/src/display/transport_stream.js b/src/display/transport_stream.js index e734b112cca12..f21c070dff283 100644 --- a/src/display/transport_stream.js +++ b/src/display/transport_stream.js @@ -41,8 +41,8 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() { this._onReceiveData({ begin, chunk, }); }); - this._pdfDataRangeTransport.addProgressListener((loaded) => { - this._onProgress({ loaded, }); + this._pdfDataRangeTransport.addProgressListener((loaded, total) => { + this._onProgress({ loaded, total, }); }); this._pdfDataRangeTransport.addProgressiveReadListener((chunk) => { @@ -77,13 +77,18 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() { }, _onProgress: function PDFDataTransportStream_onDataProgress(evt) { - if (this._rangeReaders.length > 0) { + if (evt.total === undefined && this._rangeReaders.length > 0) { // Reporting to first range reader. var firstReader = this._rangeReaders[0]; if (firstReader.onProgress) { firstReader.onProgress({ loaded: evt.loaded, }); + return; } } + let fullReader = this._fullRequestReader; + if (fullReader && fullReader.onProgress) { + fullReader.onProgress({ loaded: evt.loaded, total: evt.total, }); + } }, _onProgressiveDone() { diff --git a/web/firefoxcom.js b/web/firefoxcom.js index bea171d1a1f01..22e044801ffa5 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -269,6 +269,10 @@ PDFViewerApplication.externalServices = { break; case 'progressiveRead': pdfDataRangeTransport.onDataProgressiveRead(args.chunk); + + // Don't forget to report loading progress as well, since otherwise + // the loadingBar won't update when `disableRange=true` is set. + pdfDataRangeTransport.onDataProgress(args.loaded, args.total); break; case 'progressiveDone': if (pdfDataRangeTransport) {