Closed
Bug 1201632
Opened 9 years ago
Closed 9 years ago
iframe does not fire onload when 404 is not text/html or text/plain
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox43 | --- | affected |
People
(Reporter: nsm, Unassigned)
References
Details
Attachments
(2 files)
STR:
1. Run the test case with |node server.js|.
2. Visit localhost:8080
3. The test will add 3 iframes each of which 404s with different content types.
4. Open the console, you'll see that the onload event fires for text/plain and text/html but not for text/json. Also, the iframe's contents in the json case are not the bytes sent by the server but a custom firefox error, which is different to the other two.
Comment 1•9 years ago
|
||
> Also, the iframe's contents in the json case are not the bytes sent by the server but a
> custom firefox error
Hence no onload. Error pages don't fire any of that stuff.
Comment 2•9 years ago
|
||
Yea, this is why we have to do this evilness in the wpt tests:
https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/mozilla/tests/service-workers/service-worker/fetch-frame-resource.https.html#34
I suppose we could write an html spec issue now to try to get some kind of onerror event.
Reporter | ||
Comment 3•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #1)
> > Also, the iframe's contents in the json case are not the bytes sent by the server but a
> > custom firefox error
>
> Hence no onload. Error pages don't fire any of that stuff.
So is this fixable? i.e. what in the spec gets us to show contents for those mimetypes but custom errors for others?
Comment 4•9 years ago
|
||
The spec is pretty handwavy about the whole thing when error responses are involved.
Is this bug report about not showing the response data (instead of our own page) for text/json, or is it about error pages not firing onload?
Reporter | ||
Comment 5•9 years ago
|
||
i'm more concerned with onload since the wpt ServiceWorker tests rely on that to progress with the test https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/test-helpers.sub.js#49
Comment 6•9 years ago
|
||
(In reply to Nikhil Marathe [:nsm] (please needinfo?) from comment #5)
> i'm more concerned with onload since the wpt ServiceWorker tests rely on
> that to progress with the test
> https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/mozilla/
> tests/service-workers/service-worker/resources/test-helpers.sub.js#49
blink treats error pages as same-origin, so they may see onload events. We treat them as cross-origin.
Comment 7•9 years ago
|
||
load event has nothing to do with same-origin or cross-origin ;)
Comment 8•9 years ago
|
||
OK. So one simple option would be to try to not show an error page in this case. I don't recall what the heuristics are for that nor where they live... :(
Reporter | ||
Comment 9•9 years ago
|
||
I investigated this pretty thoroughly. What happens is that our wpt harness uses "text/json" as the Content-Type, but RFC 4627 [1] specifies the mime type for json as "application/json". In the docshell code we look up if Gecko can render certain kinds of mime types as "content" and if yes URILoader won't fail the load with NS_ERROR_FILE_NOT_FOUND, but instead try to find a handler for the mime type. We do this correctly with application/json. [2] So this isn't exactly a bug. I am going to update our test harness to use application/json.
Some cursory "research" [3] shows "text/json" seems to be reasonably common (jquery accepts it, Blink renders it). Should we bother adding "text/json" to the list of supported types?
[1]: http://www.ietf.org/rfc/rfc4627.txt
[2]: https://dxr.mozilla.org/mozilla-central/source/layout/build/nsContentDLF.h#75
[3]: http://stackoverflow.com/questions/477816/what-is-the-correct-json-content-type
Comment 10•9 years ago
|
||
We could add text/json as one of our "text types", yeah... We should really get this list into the HTML spec; right now it only says to do the textlike rendering for text/plain.
Comment 11•9 years ago
|
||
I filed a spec issue:
https://github.com/whatwg/html/issues/124
Reporter | ||
Comment 12•9 years ago
|
||
Bug 1201632 - wptserve.py should send JSON failure message with correct mime type. r?jgraham
According to the JSON RFC [1] the official mime type is "application/json". If
we do not send this, Gecko will not fire the onload event for a frame trying to
load such a URL but will instead load an error page.
Several service worker tests rely on loading 'fake' iframes that 404 but
contribute to the progress of the test by watching for the onload event.
These tests are fixed by this patch.
[1]: http://www.ietf.org/rfc/rfc4627.txt
Attachment #8658780 -
Flags: review?(james)
Comment 13•9 years ago
|
||
Comment on attachment 8658780 [details]
MozReview Request: Bug 1201632 - wptserve.py should send JSON failure message with correct mime type. r?jgraham
https://reviewboard.mozilla.org/r/18685/#review16701
Needs an upstream patch or we will lose it on the next wpt update.
Attachment #8658780 -
Flags: review?(james) → review+
Reporter | ||
Comment 14•9 years ago
|
||
Comment 15•9 years ago
|
||
Comment 16•9 years ago
|
||
Comment 17•9 years ago
|
||
The caused Web Platform Reftests, W4, and W8 to fail. Additionally, I've had to back out bug 1186833 since it's a direct dependency of this bug.
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/90f84f2af43e
Comment 18•9 years ago
|
||
Sorry, I mistakenly assumed that this would land in conjunction with the relevant metadata updates. Obviously I should have said so in the review.
Reporter | ||
Comment 19•9 years ago
|
||
(In reply to James Graham [:jgraham] from comment #18)
> Sorry, I mistakenly assumed that this would land in conjunction with the
> relevant metadata updates. Obviously I should have said so in the review.
I completely forgot about needing to fix other tests :/
Reporter | ||
Comment 20•9 years ago
|
||
Fixed in Bug 1204461 due to update to harness that landed upstream.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•