[api-minor] Remove the length parameter from getDocument#20840
Merged
timvandermeij merged 1 commit intomozilla:masterfrom Mar 15, 2026
Merged
[api-minor] Remove the length parameter from getDocument#20840timvandermeij merged 1 commit intomozilla:masterfrom
length parameter from getDocument#20840timvandermeij merged 1 commit intomozilla:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20840 +/- ##
==========================================
+ Coverage 62.53% 62.55% +0.01%
==========================================
Files 171 173 +2
Lines 121171 121194 +23
==========================================
+ Hits 75779 75817 +38
+ Misses 45392 45377 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Snuffleupagus
force-pushed
the
getDocument-rm-length
branch
3 times, most recently
from
5456a47 to
cc48e58
Compare
Collaborator
Author
|
/botio test |
Collaborator
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://54.241.84.105:8877/1397e4fa7998f61/output.txt |
Collaborator
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://54.193.163.58:8877/e99fff493d2b1aa/output.txt |
Collaborator
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/1397e4fa7998f61/output.txt Total script time: 44.89 mins
Image differences available at: http://54.241.84.105:8877/1397e4fa7998f61/reftest-analyzer.html#web=eq.log |
Collaborator
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/e99fff493d2b1aa/output.txt Total script time: 70.50 mins
Image differences available at: http://54.193.163.58:8877/e99fff493d2b1aa/reftest-analyzer.html#web=eq.log |
This is an old API-parameter that is now unused within the PDF.js project itself, and its description says that it's (partly) being used for "range requests operations". Note that the `length` API-parameter is used to set the *initial* `contentLength` in various `BasePDFStreamReader` implementations, however it's always overridden by the "Content-Length" header (sent by the server) when that one exists *and* is a valid number. While we currently fallback to the keep the initial `contentLength` otherwise, note however how in that case range requests will always be *disabled* and thus the only spot in the code-base [where `fullReader.contentLength` is necessary](https://github.com/mozilla/pdf.js/blob/873378b71830a151d3350d812bf914b673149cd6/src/core/worker.js#L230-L236) cannot actually be reached. Hence the only possible reason to use the `length` API-parameter would be for improved progress reporting[1] during streaming of PDF data in rare cases where the "Content-Length" header is missing/invalid, but the user *somehow* has information from another source about the correct `length` of the PDF document. That situation feels very much like an edge-case, but it's obviously impossible to know if someone is depending on it. However, please note that there's a work-around available for users affected by this removal: - Implement a `PDFDataRangeTransport` instance together with custom data-fetching[2], since in that case its `length`-parameter will always be used as-is. Finally, updates various `BasePDFStreamReader` implementations to only set the `_isRangeSupported` field once the headers are available (since previously we'd just overwrite the "initial" value anyway). --- [1] I.e. to avoid the "indeterminate" loadingBar being displayed in the viewer. [2] This is what e.g. the Firefox PDF Viewer uses.
Snuffleupagus
force-pushed
the
getDocument-rm-length
branch
from
cc48e58 to
09a9a7b
Compare
timvandermeij
approved these changes
Mar 15, 2026
Contributor
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an old API-parameter that is now unused within the PDF.js project itself, and its description says that it's (partly) being used for "range requests operations".
Note that the
lengthAPI-parameter is used to set the initialcontentLengthin variousBasePDFStreamReaderimplementations, however it's always overridden by the "Content-Length" header (sent by the server) when that one exists and is a valid number. While we currently fallback to the keep the initialcontentLengthotherwise, note however how in that case range requests will always be disabled and thus the only spot in the code-base wherefullReader.contentLengthis necessary cannot actually be reached.Hence the only possible reason to use the
lengthAPI-parameter would be for improved progress reporting[1] during streaming of PDF data in rare cases where the "Content-Length" header is missing/invalid, but the user somehow has information from another source about the correctlengthof the PDF document.That situation feels very much like an edge-case, but it's obviously impossible to know if someone is depending on it. However, please note that there's a work-around available for users affected by this removal:
PDFDataRangeTransportinstance together with custom data-fetching[2], since in that case itslength-parameter will always be used as-is.Finally, updates various
BasePDFStreamReaderimplementations to only set the_isRangeSupportedfield once the headers are available (since previously we'd just overwrite the "initial" value anyway).[1] I.e. to avoid the "indeterminate" loadingBar being displayed in the viewer.
[2] This is what e.g. the Firefox PDF Viewer uses.