Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api-minor] Expose response headers, when available, in PDFDocumentLoadingTask #18908

Closed

Conversation

Snuffleupagus
Copy link
Collaborator

This is a feature that's been requested a few times over the years, see e.g. issue #14630, #16341, and #18792.

Note that by placing the getResponseHeaders method in PDFDocumentLoadingTask it's possible to access the response headers even if loading failed, e.g. with MissingPDFException, which wouldn't have been possible if the method was placed elsewhere (e.g. in PDFDocumentProxy).
Given the following JSDoc comment, this also seems generally reasonable:

pdf.js/src/display/api.js

Lines 570 to 575 in 689ffda

/**
* The loading task controls the operations required to load a PDF document
* (such as network requests) and provides a way to listen for completion,
* after which individual pages can be rendered.
*/
class PDFDocumentLoadingTask {

Considering that this functionality isn't relevant in e.g. the Firefox PDF Viewer, pre-processor statements are being used to limit the code-size impact of the implementation to GENERIC-builds as much as possible.


Please note: This will further increase the maintenance/support burden of the general PDF.js library, for a feature that's completely unused within the project itself.
Hence, if you'd like to see this patch landed, please contact me privately to discuss sponsoring this work (email address can be found in my GitHub profile).

@Snuffleupagus Snuffleupagus force-pushed the api-getResponseHeaders branch 2 times, most recently from 0a55f37 to 866b3a3 Compare November 4, 2024 09:33
@Snuffleupagus Snuffleupagus force-pushed the api-getResponseHeaders branch from 866b3a3 to 8254677 Compare November 5, 2024 20:26
…oadingTask`

This is a feature that's been requested a few times over the years, see e.g. issue 14630, 16341, and 18792.

Note that by placing the `getResponseHeaders` method in `PDFDocumentLoadingTask` it's possible to access the response headers even if loading failed, e.g. with `MissingPDFException`, which wouldn't have been possible if the method was placed elsewhere (e.g. in `PDFDocumentProxy`).
Given the following JSDoc comment, this also seems generally reasonable: https://github.com/mozilla/pdf.js/blob/689ffda9df29f24e03f6e5145a096584d7f166cc/src/display/api.js#L570-L575

Considering that this functionality isn't relevant in e.g. the Firefox PDF Viewer, pre-processor statements are being used to limit the code-size impact of the implementation to GENERIC-builds as much as possible.
@Snuffleupagus
Copy link
Collaborator Author

Closing for now, since no one seem to actually need or want this.

@pavdev64
Copy link

pavdev64 commented Dec 4, 2024

Oh no, I noticed this late and don't have time to test it today. I really need this feature! Please reopen or merge!

pavdev64

This comment was marked as off-topic.

@Snuffleupagus
Copy link
Collaborator Author

I really need this feature! Please reopen or merge!

As mentioned in #18908 (comment) that will require that you, or someone else, is willing to pay actual money for this feature:

Please note: This will further increase the maintenance/support burden of the general PDF.js library, for a feature that's completely unused within the project itself.
Hence, if you'd like to see this patch landed, please contact me privately to discuss sponsoring this work (email address can be found in my GitHub profile).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants