Closed Bug 1432272 Opened 3 years ago Closed 3 years ago
fetch API is using Get
Entry Document() for base URL instead of the owning global's base URL
We currently are failing a number of WPT tests dom/fetch is using GetEntryDocument() to find a base URL instead owning global's base URL. For example, these tests explicitly test for this: https://github.com/w3c/web-platform-tests/blob/master/fetch/api/request/multi-globals/url-parsing.html The spec says to use "current settings object's API base URL" in step 4 here: https://github.com/w3c/web-platform-tests/blob/master/fetch/api/request/multi-globals/url-parsing.html Its unclear to me if that is correct spec language for this. It seems like "relevant settings object" may be more appropriate: https://html.spec.whatwg.org/multipage/webappapis.html#relevant-settings-object But I'm not 100% clear on the spec language here. Boris, I intent to flag you for review since you were reviewer on the change to use GetEntryDocument() previously and tend to have opinions on this stuff. Or feel free to pass and I'll find someone else. Thanks
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Oh, boy. ;) So there are a bunch of things going on here, including strong disagreements about how this stuff would work between different people writing specs. 1) Historically (think 90s), base URLs typically came from what we now call the "entry settings object". This is, for example, how Location works. 2) There are some people who have a project of killing off the entry settings object. This involves not using it for base URLs in new APIs, and trying to change existing APIs to use something else. See https://github.com/whatwg/html/issues/1431 3) This, of course, leads to inconsistencies in how different APIs derive the base URL, which can lead to developer confusion. Thus one could make the argument that the effort described in point 2 is misguided, unless we think we can switch _everything_ off entry settings for base URLs. 4) The use of entry settings for base URLs is slightly daft in some ways, because it leads to bizarre action-at-a-distance effects. If one were designing the web platform from scratch and actually thinking about this problem, one would never choose entry settings for base URLs. This is the impetus for the effort from point 2. 5) For the specific case of the fetch API, it used to use entry settings, but that got changed in https://github.com/whatwg/fetch/issues/367 as far as I can tell. This predated the "file bugs on UAs" policy, I think. I assume your second link was meant to be to <https://fetch.spec.whatwg.org/#dom-request>, where step 4 uses "current settings object"? If so, that should in fact be current settings, because there is no meaningful "relevant settings" in a constructor. That said, the "current settings" of the constructor becomes the "relevant settings" of the object returned from the constructor, and is the "owning global" of the to-be-returned thing in terms of our code. TL;DR: implementing what the spec currently says here makes sense, I think.
Try is looking fairly green. All mochitests and WPT have completed on at least some platforms. I'm going to go ahead with flagging.
Maybe we should uplift this for webcompat reasons since its the beginning of the 59 beta cycle.
Comment on attachment 8944558 [details] [diff] [review] Make Fetch API use the global's base URL instead of the entry document's base URL. r=bz r=me Once we have a sane threadsafe URL implementation, ideally we would just have an "api base url" hanging off of nsIGlobalObject and common up the worker and non-worker paths... Might be worth filing a followup on that pointing to this code and depending on the threadsafe URL bugs.
Attachment #8944558 - Flags: review?(bzbarsky) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b5432554c7bd Make Fetch API use the global's base URL instead of the entry document's base URL. r=bz
I'm not really sure what to say about this in the docs. This is a very spec-centric point, and I don't really understand how this would affect web developers, and how to phrase this information. Ben, can you give me a bit of help here? Thanks in advance!
Chris, its an esoteric point that perhaps most sites will never run into. To run into this: 1. You need a same-origin iframe. 2. That same-origin iframe needs to have a location with a different base URL. 3. You have to use the fetch function cross-global like `frame.contentWindow.fetch()`. 4. The URL passed to fetch needs to be relative. In the past we would resolve the relative URL against the current global. So like: let absolute = new URL(relative, window.location.href) Now we resolve the relative URL against the global that owns the fetch() function being used. So in the case I describe above, its resolved against the iframe's location. So like: let absolute = new URL(relative, frame.contentWindow.location.href) I'm not sure its really worth putting all of this in the fetch() documentation. Its almost worth having some page dedicated to the corner case of cross-global usage, although I'm not sure APIs are consistent in how they handle this stuff. The spec is trying to align new APIs to what we did in this bug. Hope that helps.
(In reply to Ben Kelly [:bkelly] from comment #11) > > Hope that helps. Thanks Ben, this is really useful. I've had a go at writing this into a short article: https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Cross-global_fetch_usage I've let a few placeholders in where I think we could probably use more information. Can you help me fill it in? Thanks again.
I guess this page is ok. I'm not sure there is a particular problem the change fixes. Its more just that we're trying to be consistent in the spec for this kind of thing now and this is the behavior we decided to go with. So the change was basically to conform to the new convention we are using in the spec. Honestly, a separate "cross global issues" page on MDN would probably be helpful. It could talk about this issue and probably more. Its an area with a lot of inconsistencies between APIs and browsers, I think. I don't feel knowledgeable enough to write that page, though. You might need to talk to Boris about it.
(In reply to Ben Kelly [:bkelly] from comment #13) > I guess this page is ok. I'm not sure there is a particular problem the > change fixes. Its more just that we're trying to be consistent in the spec > for this kind of thing now and this is the behavior we decided to go with. > So the change was basically to conform to the new convention we are using in > the spec. > > Honestly, a separate "cross global issues" page on MDN would probably be > helpful. It could talk about this issue and probably more. Its an area > with a lot of inconsistencies between APIs and browsers, I think. > > I don't feel knowledgeable enough to write that page, though. You might > need to talk to Boris about it. OK; I've updated it just to say that. And I'll talk to Boris about the cross global issues suggestion.
You need to log in before you can comment on or make changes to this bug.