Closed
Bug 1209095
Opened 9 years ago
Closed 8 years ago
FetchEvent.respondWith() should accept opaqueredirect Response whenever FetchEvent.request.redirect is "manual"
Categories
(Core :: DOM: Service Workers, defect)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: bkelly, Assigned: jdm)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
9.50 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
See: https://github.com/whatwg/fetch/issues/127 Currently we block opaqueredirect in these cases unless the request is a navigation.
Reporter | ||
Updated•8 years ago
|
Blocks: ServiceWorkers-compat
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8699260 -
Flags: review?(bkelly)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → josh
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•8 years ago
|
||
Comment on attachment 8699260 [details] [diff] [review] Accept opaqueredirection fetch results if the request redirection type is manual Review of attachment 8699260 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. Thanks! ::: dom/workers/ServiceWorkerEvents.cpp @@ +575,5 @@ > mRequestURL, modeString); > return; > } > > + if (mRequestRedirectMode != RequestRedirect::Manual && response->Type() == ResponseType::Opaqueredirect) { nit: line length ::: testing/web-platform/mozilla/tests/service-workers/service-worker/resources/fetch-event-redirect-iframe.html @@ +4,5 @@ > var data = evt.data; > fetch(new Request(data.url, data.request_init)).then(function(response) { > if (data.request_init.mode === 'no-cors' && data.redirect_dest != 'same-origin') { > + var expected_type = data.expected_type || 'opaque'; > + if (response.type === expected_type) { I believe all the test cases have an expected_type, so you shouldn't need the || clause getting the expected_type. Also, I think we should only permit this to pass if the type is opaque or opaqueredirect. So maybe change this to something like: if (response.type === data.expected_type && (response.type === 'opaque' || response.type === 'opaqueredirect')) { // success }
Attachment #8699260 -
Flags: review?(bkelly) → review+
Assignee | ||
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4fce7a071df
Assignee | ||
Comment 4•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b068d21a21087b21042d106a9fab94cab3f3bcf0 Bug 1209095 - Accept opaqueredirection fetch results if the request redirection type is manual. r=bkelly
Comment 5•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b068d21a2108
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•