Bug 110867 – XHR should fire 'abort' event when navigation interrupts a request.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mike West]

https://code.google.com/p/chromium/issues/detail?id=162817 and http://bugs.jquery.com/ticket/12964 report that interrupting an XHR request via navigation fires an 'error' event when an 'abort' event should be thrown.

[Mike West]

Created attachment 190255 [details]
Patch

[Mike West]

The trivial layout test seems to show that the behavior is already correct. Asking upstream for either confirmation that the current behavior is correct, or a better test case.

[Alexey Proskuryakov]

Comment on attachment 190255 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190255&action=review

> LayoutTests/http/tests/xmlhttprequest/navigation-should-abort-expected.txt:2
> +This is just a minimal page that we navigate to as part of testing back/forward.

This is not accurate. I suggest using a different page, that also says something helpful when opened in a browser manually.

> LayoutTests/http/tests/xmlhttprequest/navigation-should-abort.html:7
> +            testRunner.dumpBackForwardList();

I do not understand why dumping b/f list is helpful.

> LayoutTests/http/tests/xmlhttprequest/navigation-should-abort.html:11
> +        req.open("GET", "/xmlhttprequest/resources/endlessxml.php");

I'd have used a relative path here.

> LayoutTests/http/tests/xmlhttprequest/navigation-should-abort.html:21
> +            testRunner.queueLoad("/navigation/resources/otherpage.html");

Ditto (or maybe just a data: URL).

[Mike West]

(In reply to comment #3)
> (From update of attachment 190255 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190255&action=review
> 
> > LayoutTests/http/tests/xmlhttprequest/navigation-should-abort-expected.txt:2
> > +This is just a minimal page that we navigate to as part of testing back/forward.
> 
> This is not accurate. I suggest using a different page, that also says something helpful when opened in a browser manually.

Swapped out with a `data:` URL as you suggested below.

> > LayoutTests/http/tests/xmlhttprequest/navigation-should-abort.html:7
> > +            testRunner.dumpBackForwardList();
> 
> I do not understand why dumping b/f list is helpful.

The idea was to prove to myself that we're navigating. You're right though, it's likely unnecessary since we get the text of the second page in the expectation file. I'll drop it.

> > LayoutTests/http/tests/xmlhttprequest/navigation-should-abort.html:11
> > +        req.open("GET", "/xmlhttprequest/resources/endlessxml.php");
> 
> I'd have used a relative path here.

Alright.

> > LayoutTests/http/tests/xmlhttprequest/navigation-should-abort.html:21
> > +            testRunner.queueLoad("/navigation/resources/otherpage.html");
> 
> Ditto (or maybe just a data: URL).

Done.

Thanks!

[Mike West]

Created attachment 190468 [details]
Patch for landing

[Mike West]

Comment on attachment 190468 [details]
Patch for landing

CQ- for a formatting error. Bleh.

[Mike West]

Created attachment 190470 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 190470 [details]
Patch for landing

Clearing flags on attachment: 190470

Committed r144163: <http://trac.webkit.org/changeset/144163>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

This test is failing on Mac WK1:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&showExpectations=true&tests=http%2Ftests%2Fxmlhttprequest%2Fnavigation-should-abort.html

[Roger Fong]

And Windows:
http://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r144386%20(32973)/http/tests/xmlhttprequest/navigation-should-abort-pretty-diff.html

[Mike West]

(In reply to comment #11)
> And Windows:
> http://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r144386%20(32973)/http/tests/xmlhttprequest/navigation-should-abort-pretty-diff.html

These should be fixed with the patch in https://bugs.webkit.org/show_bug.cgi?id=111052.