whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.18k stars 2.71k forks source link

Why do object loads fire error events sync? #1147

Open bzbarsky opened 8 years ago

bzbarsky commented 8 years ago

Failure to parse the URL or fetch failure fires error events sync instead of queueing a task to fire it like other places (e.g. images) do. On the other hand, load events are fired async...

It's probably better to fire async, because we want to do more algorithm steps after we fire the event (the whole fallback thing) and it's nice to not worry about what the error event handler did to our state when we fired the event.

foolip commented 8 years ago

Do you mean this bit?

If that failed, fire a simple event named error at the element, then jump to the step below labeled fallback.

All of that is already in a task, so it's not sync when setting the data attribute, at least. Is the steps that fallow the main problem here, and if so would it help to just jump to set a flag that's checked at the end of the algorithm to fire the event? What do implementations actually do?

bzbarsky commented 8 years ago

I haven't figured out a good way to test this in implementations yet. I do know that at least Gecko runs these steps sync from setting data:, not off a task; I don't know what other impls do.

It's possible that running these steps sync but the events off a task is black-box equivalent to running the steps async and the events sync....

bzbarsky commented 8 years ago

And again, note the asymmetry with load events.

bzbarsky commented 8 years ago

And Chrome afaict always fires "load" here, not "error", even for invalid URIs. Testcase:

<body>
  <script>
    onmessage = function(e) {
      console.log(e.data);
    }
    var obj = document.createElement("object");
    obj.data = "http://test:test";
    obj.onerror = function() { console.log("error"); }
    obj.onload = function() { console.log("load"); }
    postMessage("task before insert", "*");
    console.log("before insert");
    document.body.appendChild(obj);
    console.log("after insert");
    postMessage("task after insert", "*");
  </script>
</body>

and note that it fires more async than "task after insert", which is not surprising because as I said above "load" does get fired off an extra task.

bzbarsky commented 8 years ago

Safari, on that testcase, also fires "load", and fires it async with the appendChild call, but fires it before either of the postMessage things, which doesn't match either Chrome or the current spec (per current spec it would fire between the two postMessage things).

annevk commented 8 years ago

@bzbarsky per the specification they use different task sources, so ordering between postMessage() and <object> is not guaranteed. (Not sure why <object> only uses the "DOM manipulation task source" though as it seemingly suggests. Would expect the networking task source to be involved at least, so there's probably bugs there too.)

bzbarsky commented 8 years ago

so ordering between postMessage() and <object> is not guaranteed.

Ah, I see. Well, we should replace the postMessage calls with something that will queue a task to the task source <object> uses and retest.