Closed
Bug 1362354
Opened 7 years ago
Closed 6 years ago
Failure in testing/web-platform/tests/XMLHttpRequest/abort-after-stop.htm
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: shawnjohnjr, Assigned: twisniewski)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 4 obsolete files)
1.65 KB,
patch
|
Details | Diff | Splinter Review | |
59 bytes,
text/x-review-board-request
|
baku
:
review+
|
Details |
XMLHttpRequest: abort event should fire when stop() method is used assert_equals: expected true but got false
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → shuang
Reporter | ||
Comment 1•7 years ago
|
||
https://github.com/w3c/web-platform-tests/issues/241
Reporter | ||
Comment 2•7 years ago
|
||
Reporter | ||
Comment 3•7 years ago
|
||
I've fixed the original issue, so right now abort event will be issued. But I can still see testharness timeout. I guess that it's because window.stop() cause test.done() failure. Hi James, Do you think it's normal after calling window.stop(), test.done()[1] can't end the test case? I even tried to move test.done() into client.onabort. Timeout also happened with Chrome[2]. [1] https://github.com/w3c/web-platform-tests/blob/master/XMLHttpRequest/abort-after-stop.htm#L24 [2] https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/TestExpectations?type=cs&q=abort-after-stop.htm&l=2205
Flags: needinfo?(james)
Reporter | ||
Updated•7 years ago
|
Attachment #8890294 -
Flags: review?(amarchesini)
Reporter | ||
Comment 4•7 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a781ee9f379dc7c0ff5e312b41b33e51b745251&selectedJob=118320883
Updated•7 years ago
|
Attachment #8890294 -
Flags: review?(amarchesini) → review+
Reporter | ||
Updated•7 years ago
|
Attachment #8890294 -
Attachment is obsolete: true
Reporter | ||
Comment 5•7 years ago
|
||
Pushed by shuang@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6ed95396bdb6 Send abort event if window.stop() is called, r=baku
Comment 7•7 years ago
|
||
Backed out for failing web-platform-test /html/browsers/browsing-the-web/navigating-across-documents/010.html: https://hg.mozilla.org/integration/mozilla-inbound/rev/0739667c02050d60fec1f965307a8a85f8c40b8b Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6ed95396bdb623610e88b4d59ed267d513ae2886&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=118440054&repo=mozilla-inbound [task 2017-07-27T10:28:32.186301Z] 10:28:32 INFO - TEST-START | /html/browsers/browsing-the-web/navigating-across-documents/010.html [task 2017-07-27T10:28:32.242834Z] 10:28:32 INFO - PID 3801 | 1501151312233 Marionette DEBUG Register listener.js for window 2147483709 [task 2017-07-27T10:28:42.323396Z] 10:28:42 INFO - [task 2017-07-27T10:28:42.323678Z] 10:28:42 INFO - TEST-UNEXPECTED-FAIL | /html/browsers/browsing-the-web/navigating-across-documents/010.html | Link with onclick form submit to javascript url with delayed document.write and href navigation - assert_equals: expected "href" but got "write" [task 2017-07-27T10:28:42.324129Z] 10:28:42 INFO - onmessage<@http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/010.html:14:5 [task 2017-07-27T10:28:42.324581Z] 10:28:42 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1412:20 [task 2017-07-27T10:28:42.325065Z] 10:28:42 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1436:20 [task 2017-07-27T10:28:42.325580Z] 10:28:42 INFO - EventHandlerNonNull*@http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/010.html:12:13 [task 2017-07-27T10:28:42.326179Z] 10:28:42 INFO - TEST-UNEXPECTED-TIMEOUT | /html/browsers/browsing-the-web/navigating-across-documents/010.html | expected OK
Flags: needinfo?(shuang)
Reporter | ||
Comment 8•7 years ago
|
||
I'm sorry that I did not notice this test case fail. I will dig into it.
Flags: needinfo?(shuang)
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(james)
Reporter | ||
Comment 9•7 years ago
|
||
Without my patch, running 010.html, I always saw js error. 0:09.82 PROCESS_OUTPUT: ProcessReader (pid:29244) "JavaScript error: javascript:(function()%20{var%20x%20=%20new%20XMLHttpRequest();%20x.open('GET',%20'blank.html?pipe=trickle(d2)',%20false);%20x.send();%20document.write('<script>parent.postMessage("write",%20"*")</script>');%20return%20'<script>parent.postMessage("click",%20"*")</script>'})(), line 1: NetworkError: A network error occurred." I wonder how is this can pass the test case.
Reporter | ||
Comment 10•7 years ago
|
||
Hmm. Since js error, then href shall be always expected.
Reporter | ||
Comment 11•7 years ago
|
||
Reporter | ||
Updated•7 years ago
|
Attachment #8890773 -
Attachment is obsolete: true
Reporter | ||
Comment 12•7 years ago
|
||
Reporter | ||
Updated•7 years ago
|
Attachment #8892346 -
Attachment is obsolete: true
Reporter | ||
Comment 13•7 years ago
|
||
Reporter | ||
Updated•7 years ago
|
Attachment #8892348 -
Attachment is obsolete: true
Reporter | ||
Comment 14•7 years ago
|
||
(In reply to Shawn Huang [:shawnjohnjr] from comment #9) > Without my patch, running 010.html, I always saw js error. > > 0:09.82 PROCESS_OUTPUT: ProcessReader (pid:29244) "JavaScript error: > javascript:(function()%20{var%20x%20=%20new%20XMLHttpRequest();%20x. > open('GET',%20'blank.html?pipe=trickle(d2)',%20false);%20x.send(); > %20document.write('<script>parent.postMessage("write",%20"*")</script>'); > %20return%20'<script>parent.postMessage("click",%20"*")</script>'})(), line > 1: NetworkError: A network error occurred." > > I wonder how is this can pass the test case. Oh. Even without my patch, originally since open() is sync version which is already deprecated, then in send() function, code returns network error due to mChannel is null, it throws network error. This leads document.write() won't be executed. So the test case doesn't actually work properly. Verdict can pass just because javascript error. Hmm. I honestly think this test case is weired and now I'm now sure how to fix this test case from xhr. With my patch, xhr sends abort event, so the script will be executed anyway, then document.write happens earlier than postMessage in href.html. I wondered why this test uses xhr sync to delay 2 seconds. Considering xhr sync support also deprecated in Chrome. I think it's not necessary to use xhr sync to delay.
Reporter | ||
Comment 15•7 years ago
|
||
"Network error" was thrown because onStopRequest calls ChangeStateToDone, which set mChannel to null, then return NS_ERROR_DOM_NETWORK_ERR in SendInternal(). [1]http://searchfox.org/mozilla-central/rev/f0e4ae5f8c40ba742214e89aba3f554da0b89a33/dom/xhr/XMLHttpRequestMainThread.cpp#2421 [2]http://searchfox.org/mozilla-central/rev/f0e4ae5f8c40ba742214e89aba3f554da0b89a33/dom/xhr/XMLHttpRequestMainThread.cpp#3125
Updated•7 years ago
|
Priority: -- → P2
Reporter | ||
Updated•6 years ago
|
Assignee: shuang → nobody
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•6 years ago
|
||
Try run seems fine, just unrelated intermittents: https://treeherder.mozilla.org/#/jobs?repo=try&revision=16c3feb601e93d870051d616f156479c14ae02a9
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8997495 [details] Bug 1362354 - Handle XHRs with aborted bindings as aborted rather than network errors. https://reviewboard.mozilla.org/r/261240/#review268710
Attachment #8997495 -
Flags: review?(amarchesini) → review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 19•6 years ago
|
||
Pushed by aiakab@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8ee7533d1e8e Handle XHRs with aborted bindings as aborted rather than network errors. r=baku
Keywords: checkin-needed
Comment 20•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8ee7533d1e8e
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•6 years ago
|
Assignee: nobody → twisniewski
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•