Closed Bug 1250985 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

Details

(Whiteboard: btpp-active)

Attachments

(2 files)

This is about implementing 12.1 under <https://fetch.spec.whatwg.org/#dom-request>.
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
(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.
https://hg.mozilla.org/mozilla-central/rev/b9fce5b8b83c
https://hg.mozilla.org/mozilla-central/rev/97636b12d458
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.