-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Use response-Headers
in the different IPDFStream
implementations
#18682
Conversation
a5dcf82
to
1ae47c4
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://54.241.84.105:8877/a2311a2c6653c74/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://54.193.163.58:8877/0b4bd8610d4a358/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/a2311a2c6653c74/output.txt Total script time: 30.60 mins
Image differences available at: http://54.241.84.105:8877/a2311a2c6653c74/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/0b4bd8610d4a358/output.txt Total script time: 45.98 mins
Image differences available at: http://54.193.163.58:8877/0b4bd8610d4a358/reftest-analyzer.html#web=eq.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, with the comment below addressed.
Given that the `Headers` functionality is now available in all browsers/environments that we support, [see MDN](https://developer.mozilla.org/en-US/docs/Web/API/Headers#browser_compatibility), we can utilize "proper" `Headers` in the helper functions that are used to parse the response.
This improves consistency in the code-base since the implementations with the Fetch API respectively Node.js uses that name.
1ae47c4
to
2a01931
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/1785984be3a6681/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/bdd22b640958b26/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/1785984be3a6681/output.txt Total script time: 30.86 mins
Image differences available at: http://54.241.84.105:8877/1785984be3a6681/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/bdd22b640958b26/output.txt Total script time: 47.15 mins
Image differences available at: http://54.193.163.58:8877/bdd22b640958b26/reftest-analyzer.html#web=eq.log |
Thanks! |
Fixes mozilla#18957 mozilla#18682 introduced a regression that causes the following error: ``` Uncaught TypeError: Failed to construct 'Headers': Invalid name at PDFNetworkStreamFullRequestReader._onHeadersReceived (pdf.mjs:10214:29) at NetworkManager.onStateChange (pdf.mjs:10103:22) ``` The mentioned PR replaced a call to `getResponseHeader()` with `getAllResponseHeaders()` without handling cases where it may return null or an empty string. Quote from the [docs](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/getAllResponseHeaders#return_value): > Returns: > >A string representing all of the response's headers (except those whose field name is Set-Cookie) separated by [CRLF](https://developer.mozilla.org/en-US/docs/Glossary/CRLF), or null if no response has been received. If a network error happened, an empty string is returned.
Fixes mozilla#18957 mozilla#18682 introduced a regression that causes the following error: ``` Uncaught TypeError: Failed to construct 'Headers': Invalid name at PDFNetworkStreamFullRequestReader._onHeadersReceived (pdf.mjs:10214:29) at NetworkManager.onStateChange (pdf.mjs:10103:22) ``` The mentioned PR replaced a call to `getResponseHeader()` with `getAllResponseHeaders()` without handling cases where it may return null or an empty string. Quote from the [docs](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/getAllResponseHeaders#return_value): > Returns: > >A string representing all of the response's headers (except those whose field name is Set-Cookie) separated by CRLF, or null if no response has been received. If a network error happened, an empty string is returned. Run the following code and observe the error in the console. Note that the URL is intentionally set to an invalid value to simulate network error ```js <script src="//mozilla.github.io/pdf.js/build/pdf.mjs" type="module"></script> <script type="module"> var url = 'blob:'; pdfjsLib.GlobalWorkerOptions.workerSrc = '//mozilla.github.io/pdf.js/build/pdf.worker.mjs'; var loadingTask = pdfjsLib.getDocument(url); loadingTask.promise .then((pdf) => console.log('PDF loaded')) .catch((reason) => console.error(reason)); </script> ```
Given that the
Headers
functionality is now available in all browsers/environments that we support, see MDN, we can utilize "proper"Headers
in the helper functions that are used to parse the response.