Closed
Bug 1346767
Opened 7 years ago
Closed 7 years ago
XMLHttpRequest: upload listener flag not implemented correctly
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: annevk, Assigned: shawnjohnjr)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 3 obsolete files)
2.75 KB,
patch
|
Details | Diff | Splinter Review | |
1.22 KB,
patch
|
Details | Diff | Splinter Review |
In particular, it seems we bypass it for same-origin requests, which is unlikely to work well, as same-origin can be redirected to cross-origin. https://github.com/w3c/web-platform-tests/pull/5122
Comment 1•7 years ago
|
||
Maybe baku knows what's up here?
Flags: needinfo?(amarchesini)
Priority: -- → P2
Assignee | ||
Comment 2•7 years ago
|
||
The test case has not landed yet and is still up for review. https://github.com/w3c/web-platform-tests/pull/5122/commits/1abc11ce5868e29e4416c7e50ae0332d5b657bf9
Assignee | ||
Comment 3•7 years ago
|
||
It seems to me an intermittent bug. Upload events registered too late (resources/redirect.py?code=307&location=http://www1.web-platform.test:8000/XMLHttpRequest/resources/corsenabled.py) assert_unreached: Reached unreachable code EventHandlerNonNull*@http://web-platform.test:8000/XMLHttpRequest/event-upload-progress-crossorigin.htm:30:32 Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1405:20 async_test@http://web-platform.test:8000/resources/testharness.js:514:13 @http://web-platform.test:8000/XMLHttpRequest/event-upload-progress-crossorigin.htm:24:3 @http://web-platform.test:8000/XMLHttpRequest/event-upload-progress-crossorigin.htm:23:2
Assignee | ||
Comment 4•7 years ago
|
||
Original bug: https://github.com/whatwg/xhr/issues/95
Assignee | ||
Comment 5•7 years ago
|
||
So that unexpected progress event is from: http://searchfox.org/mozilla-central/rev/1a0d9545b9805f50a70de703a3c04fc0d22e3839/dom/xhr/XMLHttpRequestMainThread.cpp#3699-3700
Assignee | ||
Comment 6•7 years ago
|
||
One of ProgressEventTimer doesn't get cancel, then it fires progress event unexpectedly. I will find out why progress event timer still exists. We also need to make sure if event listeners are registered after send(), progress event should not fire at all.
Updated•7 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → shuang
Assignee | ||
Comment 7•7 years ago
|
||
Sorry for late response, now I switch back to this bug. Set ETA.
Whiteboard: [ETA:6/20]
Updated•7 years ago
|
Flags: needinfo?(amarchesini)
Assignee | ||
Updated•7 years ago
|
Whiteboard: [ETA:6/20]
Assignee | ||
Comment 8•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8888782 -
Attachment is obsolete: true
Assignee | ||
Comment 9•7 years ago
|
||
Assignee | ||
Comment 10•7 years ago
|
||
Assignee | ||
Comment 11•7 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4557f3fe1619ab4972f26d5638f14094336ce5c7
Assignee | ||
Comment 12•7 years ago
|
||
Not sure if this could break other tests compatibility, so run try all. The risk is that xhr.upload event listeners are registered after xhr.send() in same origin/non-simple, progress events shall not be received, maybe this would break some web sites.
Assignee | ||
Comment 13•7 years ago
|
||
(In reply to Shawn Huang [:shawnjohnjr] from comment #11) > try: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=4557f3fe1619ab4972f26d5638f14094336ce5c7 There are so many unknown error. Push to try again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae85a618859475927bfb95e1f5d37a8070dea578
Assignee | ||
Updated•7 years ago
|
Attachment #8888785 -
Flags: review?(amarchesini)
Assignee | ||
Updated•7 years ago
|
Attachment #8888786 -
Flags: review?(amarchesini)
Updated•7 years ago
|
Attachment #8888785 -
Flags: review?(amarchesini) → review+
Updated•7 years ago
|
Attachment #8888786 -
Flags: review?(amarchesini) → review+
Assignee | ||
Updated•7 years ago
|
Attachment #8888785 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8888786 -
Attachment is obsolete: true
Assignee | ||
Comment 14•7 years ago
|
||
Assignee | ||
Comment 15•7 years ago
|
||
Comment 16•7 years ago
|
||
Pushed by shuang@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f0817bf2f80b Part 1: Check mFlagHadUploadListenersOnSend before sending progress event, r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/4377bafde262 Part 2: Make sure xhr.upload event listeners will be ignored if listeners are registered after send(), r=baku
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f0817bf2f80b https://hg.mozilla.org/mozilla-central/rev/4377bafde262
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 18•7 years ago
|
||
Ah wait, doesn't look like I need to update any docs to account for this.
Keywords: dev-doc-needed
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
•