814576 - "abort" listener listening only for trusted events not executed for XHR if it's triggered synchronously by content calling abort()

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Test case

I am registering a listener for "abort" event in XHR
xhr.addEventListener("abort", onAbort, false);
but it isn't executed in the following case:

    var request = new XMLHttpRequest();
    request.open("POST", "issue2462.php", true);
    request.send(null);
    request.abort();

nsIEventListenerService.addListenerForAllEvents doesn't work either

Related discussion:
http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/1789488182daa387/98c98492cba74c63?hl=en#98c98492cba74c63

---

You need to test with this patch applied:
https://bug800799.bugzilla.mozilla.org/attachment.cgi?id=683848

STR:

1) Install the attached extension (the source is here: https://github.com/firebug/manual-tests/tree/master/mozilla/noabort)

2) Open: https://getfirebug.com/tests/head/console/spy/2462/issue2462.html

3) Watch Firefox Error Console or OS system console. "readystatechnage" event is sent, but I don't see "abort" -> BUG

---

Possibly related:
Bug 802551 - onHTTPSpyReadyStateChange not sent for aborted XHR
Bug 800799 - http-on-modify-request now called after JS that started the load is no longer on the stack


Honza

Comment 1

[Boris Zbarsky [:bzbarsky]]

What happens here is that nsXMLHttpRequest::Abort ends up calling nsXMLHttpRequest::DispatchProgressEvent which calls nsXMLHttpRequest::DispatchDOMEvent which ends up calling nsEventDispatcher::DispatchDOMEvent eventually.

The event is marked trusted, so nsEventDispatcher::DispatchDOMEvent goes ahead and does this:

  aDOMEvent->SetTrusted(nsContentUtils::IsCallerChrome());

But "caller" in this case is the page calling abort(), so the event is marked untrusted.  And the event listeners here are listening to trusted events only, so don't get this event.

Olli, it's weird that we're marking this event untrusted.  Should XHR be pushing a null JSContext on the stack somewhere here?  Or should we have some way to keep DispatchDOMEvent from clobbering the trusted status of the event?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

XHR should mark the event trusted.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

DispatchDOMEvent doesn't clobber the trusted event status if the event hasn't be dispatched before and it is already trusted.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> XHR should mark the event trusted.

It does. Quoting from nsXMLHttpRequest::DispatchProgressEvent:

    event->SetTrusted(true);

> DispatchDOMEvent doesn't clobber the trusted event status

Hmm.  Right you are.

Looks like the real issue is that the SetTrusted call happens before InitProgressEvent(), which clears the trusted flag?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Make XMLHttpRequest really set its progress events as trusted.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 685694 [details] [diff] [review]
Make XMLHttpRequest really set its progress events as trusted.

Thanks!

Comment 7

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4909612f296

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d4909612f296

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Testing with Firebug and works great, thanks!
Honza

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Any chance this would be ported to Fx19?

Honza

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 685694 [details] [diff] [review]
Make XMLHttpRequest really set its progress events as trusted.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Doesn't obviously seem to be a
   regression.
User impact if declined: Some aspects of Firebug don't work correctly on some
   pages.
Testing completed (on m-c, etc.): Manual and automated testing, but for Firefox
   and Firebug.
Risk to taking this patch (and alternatives if risky): Very low risk, in both my
   opinion and Olli's
String or UUID changes made by this patch: None.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 685694 [details] [diff] [review]
Make XMLHttpRequest really set its progress events as trusted.

Low risk and landing before 18 beta 4 so we have time to backout if there are any surprises here.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/180a31b074f0

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Excellent, thanks!
Honza