From d25f13d1fd6d7fc10845ee15c045770aa72ad418 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 1 Feb 2026 18:20:19 +0100 Subject: [PATCH] Report loading progress "automatically" when using the `PDFDataTransportStream` class, and remove the `PDFDataRangeTransport.prototype.onDataProgress` method This is consistent with the other `BasePDFStream` implementations, and simplifies the API surface of the `PDFDataRangeTransport` class (note the changes in the viewer). Given that the `onDataProgress` method was changed to a no-op this won't affect third-party users, assuming there even are any since this code was written specifically for the Firefox PDF Viewer. --- src/display/api.js | 33 +++++++++++------------------ src/display/fetch_stream.js | 5 +---- src/display/node_stream.js | 7 ++----- src/display/transport_stream.js | 17 +++++++++------ src/shared/base_pdf_stream.js | 4 ++++ test/unit/api_spec.js | 37 ++++++++++++++++++++++++++++++++- web/firefoxcom.js | 7 ------- 7 files changed, 66 insertions(+), 44 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index f088b9dc1..9ffb982f0 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -639,8 +639,6 @@ class PDFDataRangeTransport { #progressiveReadListeners = []; - #progressListeners = []; - #rangeListeners = []; /** @@ -659,6 +657,18 @@ class PDFDataRangeTransport { this.initialData = initialData; this.progressiveDone = progressiveDone; this.contentDispositionFilename = contentDispositionFilename; + + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { + Object.defineProperty(this, "onDataProgress", { + value: () => { + deprecated( + "`PDFDataRangeTransport.prototype.onDataProgress` - method was " + + "removed, since loading progress is now reported automatically " + + "through the `PDFDataTransportStream` class (and related code)." + ); + }, + }); + } } /** @@ -668,13 +678,6 @@ class PDFDataRangeTransport { this.#rangeListeners.push(listener); } - /** - * @param {function} listener - */ - addProgressListener(listener) { - this.#progressListeners.push(listener); - } - /** * @param {function} listener */ @@ -699,18 +702,6 @@ class PDFDataRangeTransport { } } - /** - * @param {number} loaded - * @param {number|undefined} total - */ - onDataProgress(loaded, total) { - this.#capability.promise.then(() => { - for (const listener of this.#progressListeners) { - listener(loaded, total); - } - }); - } - /** * @param {Uint8Array|null} chunk */ diff --git a/src/display/fetch_stream.js b/src/display/fetch_stream.js index c1b1b636d..5732f6085 100644 --- a/src/display/fetch_stream.js +++ b/src/display/fetch_stream.js @@ -135,10 +135,7 @@ class PDFFetchStreamReader extends BasePDFStreamReader { return { value, done }; } this._loaded += value.byteLength; - this.onProgress?.({ - loaded: this._loaded, - total: this._contentLength, - }); + this._callOnProgress(); return { value: getArrayBuffer(value), done: false }; } diff --git a/src/display/node_stream.js b/src/display/node_stream.js index 35dfaefdf..6041bc3f8 100644 --- a/src/display/node_stream.js +++ b/src/display/node_stream.js @@ -129,11 +129,8 @@ class PDFNodeStreamReader extends BasePDFStreamReader { if (done) { return { value, done }; } - this._loaded += value.length; - this.onProgress?.({ - loaded: this._loaded, - total: this._contentLength, - }); + this._loaded += value.byteLength; + this._callOnProgress(); return { value: getArrayBuffer(value), done: false }; } diff --git a/src/display/transport_stream.js b/src/display/transport_stream.js index 57423932b..325eac676 100644 --- a/src/display/transport_stream.js +++ b/src/display/transport_stream.js @@ -53,12 +53,6 @@ class PDFDataTransportStream extends BasePDFStream { this.#onReceiveData(begin, chunk); }); - pdfDataRangeTransport.addProgressListener((loaded, total) => { - if (total !== undefined) { - this._fullReader?.onProgress?.({ loaded, total }); - } - }); - pdfDataRangeTransport.addProgressiveReadListener(chunk => { this.#onReceiveData(/* begin = */ undefined, chunk); }); @@ -144,6 +138,16 @@ class PDFDataTransportStreamReader extends BasePDFStreamReader { this._filename = contentDispositionFilename; } this._headersCapability.resolve(); + + // Report loading progress when there is `initialData`, and `_enqueue` has + // not been invoked, but with a small delay to give an `onProgress` callback + // a chance to be registered first. + const loaded = this._loaded; + Promise.resolve().then(() => { + if (loaded > 0 && this._loaded === loaded) { + this._callOnProgress(); + } + }); } _enqueue(chunk) { @@ -157,6 +161,7 @@ class PDFDataTransportStreamReader extends BasePDFStreamReader { this._queuedChunks.push(chunk); } this._loaded += chunk.byteLength; + this._callOnProgress(); } async read() { diff --git a/src/shared/base_pdf_stream.js b/src/shared/base_pdf_stream.js index 4f3ee4dfe..4db338bd6 100644 --- a/src/shared/base_pdf_stream.js +++ b/src/shared/base_pdf_stream.js @@ -128,6 +128,10 @@ class BasePDFStreamReader { this._stream = stream; } + _callOnProgress() { + this.onProgress?.({ loaded: this._loaded, total: this._contentLength }); + } + /** * Gets a promise that is resolved when the headers and other metadata of * the PDF data stream are available. diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 801577518..1d619f2cb 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -5180,6 +5180,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) it("should fetch document info and page using ranges", async function () { const initialDataLength = 4000; const subArrays = []; + let initialProgress = null; let fetches = 0; const data = await dataPromise; @@ -5193,12 +5194,16 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) const chunk = new Uint8Array(data.subarray(begin, end)); subArrays.push(chunk); - transport.onDataProgress(initialDataLength); transport.onDataRange(begin, chunk); }); }; const loadingTask = getDocument({ range: transport }); + loadingTask.onProgress = evt => { + initialProgress = evt; + loadingTask.onProgress = null; + }; + const pdfDocument = await loadingTask.promise; expect(pdfDocument.numPages).toEqual(14); @@ -5206,6 +5211,12 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) expect(pdfPage.rotate).toEqual(0); expect(fetches).toBeGreaterThan(2); + expect(initialProgress).toEqual({ + loaded: initialDataLength, + total: data.length, + percent: 0, + }); + // Check that the TypedArrays were transferred. for (const array of subArrays) { expect(array.length).toEqual(0); @@ -5217,6 +5228,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) it("should fetch document info and page using range and streaming", async function () { const initialDataLength = 4000; const subArrays = []; + let initialProgress = null; let fetches = 0; const data = await dataPromise; @@ -5242,6 +5254,11 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) }; const loadingTask = getDocument({ range: transport }); + loadingTask.onProgress = evt => { + initialProgress = evt; + loadingTask.onProgress = null; + }; + const pdfDocument = await loadingTask.promise; expect(pdfDocument.numPages).toEqual(14); @@ -5249,6 +5266,12 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) expect(pdfPage.rotate).toEqual(0); expect(fetches).toEqual(1); + expect(initialProgress).toEqual({ + loaded: initialDataLength, + total: data.length, + percent: 0, + }); + await new Promise(resolve => { waitSome(resolve); }); @@ -5266,6 +5289,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) "using complete initialData", async function () { const subArrays = []; + let initialProgress = null; let fetches = 0; const data = await dataPromise; @@ -5285,6 +5309,11 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) disableRange: true, range: transport, }); + loadingTask.onProgress = evt => { + initialProgress = evt; + loadingTask.onProgress = null; + }; + const pdfDocument = await loadingTask.promise; expect(pdfDocument.numPages).toEqual(14); @@ -5292,6 +5321,12 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) expect(pdfPage.rotate).toEqual(0); expect(fetches).toEqual(0); + expect(initialProgress).toEqual({ + loaded: data.length, + total: data.length, + percent: 100, + }); + // Check that the TypedArrays were transferred. for (const array of subArrays) { expect(array.length).toEqual(0); diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 8db4d8965..81ec67985 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -613,15 +613,8 @@ class ExternalServices extends BaseExternalServices { case "range": pdfDataRangeTransport.onDataRange(args.begin, args.chunk); break; - case "rangeProgress": - pdfDataRangeTransport.onDataProgress(args.loaded); - 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": pdfDataRangeTransport?.onDataProgressiveDone();