Closed Bug 1126815 Opened 5 years ago Closed 5 years ago

implement Fetch Response finalURL attribute

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: bkelly, Assigned: nsm)

References

Details

Attachments

(1 file)

The fetch Response object recently added a FinalURL attribute:

  https://github.com/whatwg/fetch/commit/1ed10e81fe2301eea9974d8f8c671aae36bdd487
Blocks: 1126818
Assignee: nobody → nsm.nikhil
Assignee: nsm.nikhil → nobody
Mentor: nsm.nikhil
Whiteboard: [good first bug][lang=C++]
Hello, i am not getting idea what needs to be done. Any help?
(In reply to surabhi anand from comment #1)
> Hello, i am not getting idea what needs to be done. Any help?

The fetch spec added a boolean flag finalURL https://fetch.spec.whatwg.org/#concept-response-final-url-flag
To implement it in Gecko, you'll have to add the same to WebIDL at dom/webidl/Response.webidl.
You will also have to add a boolean member to dom/fetch/InternalResponse.cpp, and a getter+setter for it, along with a getter and setter in dom/fetch/Response.cpp that just forwards to the internal response. Then, based on how the spec defines the usage of it, behaviour will have to be implemented.
(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #2)
Please correct me -


> The fetch spec added a boolean flag finalURL
> https://fetch.spec.whatwg.org/#concept-response-final-url-flag
> To implement it in Gecko, you'll have to add the same to WebIDL at
> dom/webidl/Response.webidl.

1-What i have understood so far by reading fetch spec that this finalURL attribute has to return true if the response's final url flag is set ,and this response is the URL of the requested thing whatever is being fetched ?

> You will also have to add a boolean member to
> dom/fetch/InternalResponse.cpp, and a getter+setter for it, along with a
> getter and setter in dom/fetch/Response.cpp that just forwards to the
> internal response.

2-This finalURL is a boolean flag but what is this final url flag ,i mean i have to set or unset it but by what(probably URL) ?

> Then, based on how the spec defines the usage of it, behaviour will have to be implemented.

3-How do i have to implement it,first in WebIDL ?.And by behaviour do you mean the way this flag returns true or false according to the state of final url flag ?
The fetch spec's lower case request/response concepts map to dom/fetch/Internal{Request,Response}, while the title-case, DOM-exposed Request/Response map to dom/fetch/{Request,Response}. So you will need a corresponding final url flag on InternalResponse. 

For 3, yes I mean what you stated. Adding a webidl attribute will only generate some automatic code that will call Response::FinalURL() and Response::SetFinalURL(bool) methods, which you'll have to define. Those methods will just get/set the value of the underlying InternalResponse (Response has mInternalResponse). In dom/fetch/FetchDriver.cpp, you'll have to implement the behaviour specified in Fetch step 9.
Assignee: nobody → san13692
status-b2g-v2.1: --- → ?
status-b2g-v2.1: ? → ---
Sorry Nikhil i cant work on this bug right now.Thank you:)
Assignee: san13692 → nobody
Assignee: nobody → nsm.nikhil
Mentor: nsm.nikhil
Whiteboard: [good first bug][lang=C++]
Comment on attachment 8565543 [details] [diff] [review]
Implement Response.finalURL

Review of attachment 8565543 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.  Review to baku for DOM peer sign off.

Thanks!

::: dom/webidl/Response.webidl
@@ +19,5 @@
>  
>    readonly attribute USVString url;
> +  [Throws]
> +           attribute boolean finalURL;
> +

Nit: remove extra line to minimize diff to spec

::: dom/workers/test/fetch/worker_test_response.js
@@ +66,5 @@
>    ok(!r4.ok, "Response with status 302 should have ok false");
>  }
>  
> +// It is not possible to test setting finalURL until we have ServiceWorker
> +// interception. This is because synthetic Responses do not have a url, the url

Can we file a bug or reference an existing bug to test that finalURL is set correctly in the future?
Attachment #8565543 - Flags: review?(bkelly)
Attachment #8565543 - Flags: review?(amarchesini)
Attachment #8565543 - Flags: review+
Attachment #8565543 - Flags: review?(amarchesini) → review+
https://hg.mozilla.org/mozilla-central/rev/4b55612b43f5
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.