Do not fire error events on inactive globals #1995
Conversation
That is, on globals whose responsible document is not fully active. Discussed in #1956.
domenic
added
do not merge yet
|
Wrote tests for the worker case as another commit in web-platform-tests/wpt#4089. Assuming I did them correctly, they show that Firefox and Edge have some kind of check that prevents the error events from firing, whereas Chrome lets them through. |
|
@domenic You mean http://w3c-test.org/submissions/4089/workers/Worker_ErrorEvent_after_navigation.htm ? I see it report PASS twice in Chrome. That said, the test seems to be asserting that the onerror of the window after navigation never fires? Why would it fire? I'm not quite sure what the test is trying to test.... Github is giving me a 500 error on web-platform-tests/wpt#4089 as long as I'm signed in, so I guess I'll just put my other comments here: You can presumably use assert_equals as |
|
@bzbarsky you're right on all counts. I think I fixed the tests. Let me know what you think of them/the spec. |
domenic
removed
the
do not merge yet
|
|
||
| <ol> | ||
|
|
||
| <li><p>If <var>target</var> is <span>in error reporting mode</span>, then abort these | ||
| steps; the error is <i data-x="concept-error-nothandled">not handled</i>.</p></li> | ||
|
|
||
| <li><p>If <var>target</var>'s <span>responsible <code>Document</code></span> is not <span>fully |
There was a problem hiding this comment.
A few issues here:
- target is a global object, right? Those don't have a "responsible document". Settings objects do.
- Only settings objects for windows have one; does this spec algorithm ever get run for non-window globals?
- Do UAs actually do the "fully active" thing? I know Gecko has no such concept, so I expect that in practice it doesn't do what this spec says to do; if other UAs don't either, that makes web compat a worry.
- Does anything define, at this point, what should happen for the "remove an iframe from the DOM" case? It would be really nice to at least have tests for it.
There was a problem hiding this comment.
- Right. In light of (2), I guess I should have just gone with target's associated Document.
- Oops, I thought that workers had responsible documents too, but I didn't check. Of course they don't. This algorithm does get run for workers too. I was trying to test it, under the assumption that the "creator document" (not a real concept I guess) was consulted. But maybe all I was testing is that the worker gets terminated on navigation or something. Which is probably racey :-/. What would you recommend? Just skip this step for non-Windows?
- I'm not sure. I think Servo does given some discussion on session history tracking. As I said in the OP, I mostly picked this because it's what much of the rest of the spec uses :-/.
- That discards the browsing context, which means that per https://html.spec.whatwg.org/#concept-document-bc, the document no longer has a browsing context.
|
I'm happy to keep working on this, but it's a little discouraging because it seems like it's just a quagmire of underspecified and undertested behavior that keeps getting worse the more we dig into it, necessitating more tests being written and more spec text that we're unsure about, all for somewhat minimal gain. So I guess it is lower priority for me and I might have trouble getting it over the finish line. |
|
I agree about the underspecified/undertested morass here. I'm just being a bit worried about specifying something that is not web-compatible... I'll respond to the substance of #1995 (comment) when I have had a bit more time to think about it, tomorrow. I also plan to carefully look through the tests in web-platform-tests/wpt#4089 then. I totally understand it if you're not terribly motivated to keep pushing on this one; I wonder whether we can get someone to pick up writing some tests around this stuff, at least, so we'd have an idea of what current UAs do. |
That is, on globals whose responsible document is not fully active.
Discussed in #1956.
Marking "do not merge yet" as I have two questions. We should discuss them here, since #1956 was actually about something different.
💥 Error: Wattsi server error 💥
PR Preview failed to build. (Last tried on Jan 15, 2021, 7:57 AM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 Wattsi Server - Wattsi Server is the web service used to build the WHATWG HTML spec.
🔗 Related URL
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.