Closed
Bug 1302090
Opened 8 years ago
Closed 7 years ago
new Response with additional headers not respected
Categories
(Core :: DOM: Service Workers, defect, P2)
Tracking
()
RESOLVED
INCOMPLETE
People
(Reporter: wheresrhys, Unassigned)
References
(Blocks 1 open bug)
Details
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36 Steps to reproduce: The following function takes a Response and a map of headers and returns a new Response with the original Response's headers augmented by the new map. Note that it does not use Response.clone(), but rather creates a wholly new Response object. when running integration tests this function is called on each cors response in order to indicate the response was retrieved from the cache ``` function addHeadersToResponse (res, headers) { const response = res.clone() const init = { status: response.status, statusText: response.statusText, headers }; response.headers.forEach((v,k) => { init.headers[k] = v; }); return response.text() .then(body => { return new Response(body, init); }) } ``` Actual results: In Chrome this function works as expected. However in firefox 48.0.2 the new Response is created, and logging verifies that this is passed in to `event.respondWith()`, however when the response passed to the page is logged it appears to be the original response retrieved from the cache as the new headers have not been added Expected results: Additional headers should have been present in the response in the client page.
Reporter | ||
Comment 1•8 years ago
|
||
Note: when I said res.clone() is not used, I meant that no attempt is made to mutate the headers of res.clone() and respond with them (which should not work as per the spec)
Updated•8 years ago
|
Component: Untriaged → Networking
Product: Firefox → Core
Comment 2•8 years ago
|
||
What kind of headers are you trying to apply? Is there a URL where I can see this failing?
Flags: needinfo?(wheresrhys)
Updated•8 years ago
|
Component: Networking → DOM: Service Workers
Reporter | ||
Comment 3•8 years ago
|
||
The header is `FT-Debug: true` If you check out this branch https://github.com/Financial-Times/n-service-worker/tree/rhys/reduced-firefox-test, run `npm install && make -j2 test-chrome test-firefox` you should see the different results (NOTE - the test bootstrapping is a little buggy in this branch, so if you see failures in both browsers try clearing idb and the cache api and unregistering the sw)
Comment 4•8 years ago
|
||
How do I build this on windows using just node/npm? Or can you just package up the resulting dist directory and attach it here?
Reporter | ||
Comment 5•8 years ago
|
||
The make task just calls karma - a node process - with some options, so you can try running `karma start --autoWatch=true --singleRun=false --browsers=Firefox` in your terminal. If that doesn't work I'll try saving the karma test page and uploading as a zip
Comment 6•7 years ago
|
||
Did you ever get this built locally, Ben? Did anything change in the meantime in our Response implementation?
Flags: needinfo?(bkelly)
Updated•7 years ago
|
Blocks: ServiceWorkers-compat
Priority: -- → P2
Comment 7•7 years ago
|
||
I'm going to close this. We were never able to reproduce. Happy to investigate if the problem can be shown with a glitch.com example or something.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(wheresrhys)
Flags: needinfo?(bkelly)
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•