Prevent "copy constructing" a Request object with navigate mode if a RequestInit member is present

RESOLVED FIXED in Firefox 47

Status

()

Core
DOM
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Away for a while, Assigned: Away for a while)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
This is about implementing 12.1 under <https://fetch.spec.whatwg.org/#dom-request>.
(Assignee)

Comment 1

2 years ago
Created attachment 8723147 [details] [diff] [review]
Part 1: Add a way to identify whether a WebIDL dictionary has any members present
Attachment #8723147 - Flags: review?(bzbarsky)
(Assignee)

Comment 2

2 years ago
Created attachment 8723148 [details] [diff] [review]
Part 2: Prevent copy constructing a Request object with navigate mode if a RequestInit member is present
Attachment #8723148 - Flags: review?(bzbarsky)
Comment on attachment 8723147 [details] [diff] [review]
Part 1: Add a way to identify whether a WebIDL dictionary has any members present

This will get the indent wrong for the "has default value" case, no?

You really do want to do this appending right after the conversionReplacements dictionary is set up (or _as_ it's set up, really) and before the if/elif/else bit messes with the indentation of conversionReplacements["convert"].  And then you don't need to add your own indent().

r=me with that fixed.
Attachment #8723147 - Flags: review?(bzbarsky) → review+
Comment on attachment 8723148 [details] [diff] [review]
Part 2: Prevent copy constructing a Request object with navigate mode if a RequestInit member is present

Perhaps add a test that passing undefined, null, {}, or { someUnrelatedProperty: 5 } as the second arg all allow the Request constructor to succeed in this case?

I guess if we don't do the respondWith thing we'll just get a page (a 404? or not?) which has a falsy .test_result?

r=me
Attachment #8723148 - Flags: review?(bzbarsky) → review+
Whiteboard: btpp-active
(Assignee)

Comment 5

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #4)
> I guess if we don't do the respondWith thing we'll just get a page (a 404?
> or not?) which has a falsy .test_result?

Yeah if we don't do a respondWith(), the request goes to the network, which turns into a 404 as you suspected.

Will address the rest of the comments when landing.

Comment 6

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9fce5b8b83c
https://hg.mozilla.org/integration/mozilla-inbound/rev/97636b12d458

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9fce5b8b83c
https://hg.mozilla.org/mozilla-central/rev/97636b12d458
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.