Bug 19470 - Event firing sequence on abort() after send()
Event firing sequence on abort() after send()
Status: RESOLVED FIXED
Product: WebAppsWG
Classification: Unclassified
Component: XHR
unspecified
PC Windows 3.1
: P2 normal
: ---
Assigned To: Anne
public-webapps-bugzilla
:
: 16295 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-11 13:12 UTC by Dominik Röttsches (drott)
Modified: 2013-02-20 12:01 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dominik Röttsches (drott) 2012-10-11 13:12:28 UTC
Looking at the following simplified test case:

function testAbort()
{
    xhr =  new XMLHttpRequest();
    xhr.onloadstart = <push this event to a stack>;
    xhr.onabort = <push this event to a stack>;
    xhr.onerror = <push this event to a stack>;
    xhr.onload = <push this event to a stack>;
    xhr.onloadend = <push this event to a stack>;
    xhr.onreadystatechange = function(e) {
        if (xhr.readyState == xhr.DONE)
            xhr.abort();
    }
    xhr.open("GET", "get.txt", false);
    xhr.send();
    completeTest(); // <compare stack with expected event sequence>
}

We have a synchronous GET request which is sent out to the network. For the purpose of this example, let's assume the request completes successfully, then we will end up at the rule for "Switch to the DONE state."
Citing from "Infrastructure for the send() method"
"When it is said to switch to the DONE state run these steps:
1. If the synchronous flag is set, update the response entity body.
2. Unset the synchronous flag.
3. Change the state to DONE.
4. Fire an event named readystatechange.
5. Fire a progress event named progress.
6. Fire a progress event named load.
7. Fire a progress event named loadend."


So, when executing step 4 "Fire an event named readystatechange" we come to our example test's line
        if (xhr.readyState == xhr.DONE)
            xhr.abort();
So, we call abort() downstream from the callback in step 4.

Then, 4.7.8 The abort() method says in step 1:
1. Terminate the send() algorithm.

This rule would strictly speaking abort steps 5 to 7. No more progress, load and loadend callbacks after abort(). Note: No abort event itself would be send either, since we're in DONE state.

Current behavior in WebKit is: after readystatechange: abort, loadend (which I am planning to change to "no events dispatched at all")
IE9: no events dispatched after readystatechange.
FF: After readystatechange: Load, loadend (no abort).

I don't have a fix suggestion yet, first I'd like to hear editors' feedback whether you see an issue here as well.

What is the intention of the spec - should those step 5-7 events be fired in any case?
Comment 1 Anne 2012-10-11 13:39:37 UTC
Yeah, there is a similar case with loadstart and invoking .abort() from its callback and whether that would prevent loadstart from dispatching on .upload.

The intention of the current definition is that indeed any further steps are canceled. Whether that is a workable definition or the best definition I do not know. It probably does not matter much.
Comment 2 Dominik Röttsches (drott) 2012-10-11 13:48:59 UTC
In Alexey's opinion, there should still be loadends fired at least, since there's been network traffic which comes to an end. Loadend would serve as a general purpose cleanup entry point, cmp:
https://bugs.webkit.org/show_bug.cgi?id=98404#c13
Comment 3 Anne 2012-10-11 13:59:41 UTC
So maybe instead we should have "terminate send() flag" or some such that is checked at various points in the send() algorithm?

open() can do the same thing by the way, to both abort() and send(). If you or ap have suggestions for sensible places where these flags should be checked I can update the specification to make this whole vagueness go away.
Comment 4 Anne 2012-11-21 20:41:52 UTC
*** Bug 16295 has been marked as a duplicate of this bug. ***
Comment 6 Dominik Röttsches (drott) 2013-02-19 12:51:13 UTC
Julian, Jungkee, are you going to merge this change? Thanks.
Comment 7 Julian Aubourg 2013-02-19 12:59:44 UTC
Yes, we're discussing this by mail actually ;) Thanks for the report and thank Anne for the prompt fix :)
Comment 8 Anne 2013-02-19 16:02:58 UTC
So actually Dominik, I think what Gecko does, given comment 0 is correct, is correct. Invoking abort() should not change events dispatched as part of the same task, I think. If you invoke abort() when fetching is done (as this.DONE indicates) you're simply too late.

What I fixed in the specification is the ambiguity we had before.
Comment 9 Dominik Röttsches (drott) 2013-02-19 16:25:47 UTC
(In reply to comment #8)
> So actually Dominik, I think what Gecko does, given comment 0 is correct, is
> correct. Invoking abort() should not change events dispatched as part of the
> same task, I think. If you invoke abort() when fetching is done (as
> this.DONE indicates) you're simply too late.
> 
> What I fixed in the specification is the ambiguity we had before.

Agree - Yes, it's clear now. I played computer this morning and went through the sequence again. So instead of terminating and thus basically skipping steps 5-7 of comment 0, they do get executed now and the ambiguity is removed. Abort() itself fires no events, since we're already in DONE and the substeps there are skipped.
Comment 10 Jungkee Song 2013-02-20 10:13:10 UTC
Agree on NOT removing the events dispatched from steps 5-7 of <a href="show_bug.cgi?id=19470#c0">comment 0</a>.

Anne,
In order to allow that, shouldn't the step 3 in <a href="http://xhr.spec.whatwg.org/#the-abort()-method">abort() algorithm</a> be put under step 4 as a sub step? (because progress, load, loaded would be already in task queue by that time?)
Comment 11 Dominik Röttsches (drott) 2013-02-20 10:20:33 UTC
(In reply to comment #10)
> Agree on NOT removing the events dispatched from steps 5-7 of <a
> href="show_bug.cgi?id=19470#c0">comment 0</a>.
> 
> Anne,
> In order to allow that, shouldn't the step 3 in <a
> href="http://xhr.spec.whatwg.org/#the-abort()-method">abort() algorithm</a>
> be put under step 4 as a sub step? (because progress, load, loaded would be
> already in task queue by that time?)

Are the events from step 8-9 of 
http://xhr.spec.whatwg.org/#the-send()-method actually going to a task queue? Is the "queuing to a task queue" implicit here? I am asking because there is no explicit "_enqueue_ firing an event xyz" there.
Comment 12 Anne 2013-02-20 10:29:39 UTC
No, the task that dispatches those events is ongoing and is not in a queue anymore. It's a single task that dispatches all of them.
Comment 13 Julian Aubourg 2013-02-20 11:59:33 UTC
Just a small follow-up question so that everything is clear: what are the fields of the xhr object like within those handlers that are executed after the abort? No  more meaningful status, statusText and responseXXX fields? No more headers from getResponseHeader()? Right?

It's an edge case but there is some kind of inconsistency in having the events called with an xhr that isn't in the right state anymore / doesn't provide the necessary information.

At the very least, the spec should make this abundantly clear because I don't think many people would expect that.
Comment 14 Anne 2013-02-20 12:01:23 UTC
That is clear. readyState returns UNSENT (per abort()) and the others return failure-like things because the error flag is set.