Closed Bug 1346767 Opened 3 years ago Closed 2 years ago
Request: upload listener flag not implemented correctly
Bug 1346767 - Part 2: Make sure xhr.upload event listeners will be ignored if listeners are registered after send(), r=baku
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
Maybe baku knows what's up here?
Priority: -- → P2
The test case has not landed yet and is still up for review. https://github.com/w3c/web-platform-tests/pull/5122/commits/1abc11ce5868e29e4416c7e50ae0332d5b657bf9
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
Original bug: https://github.com/whatwg/xhr/issues/95
So that unexpected progress event is from: http://searchfox.org/mozilla-central/rev/1a0d9545b9805f50a70de703a3c04fc0d22e3839/dom/xhr/XMLHttpRequestMainThread.cpp#3699-3700
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.
Assignee: nobody → shuang
Sorry for late response, now I switch back to this bug. Set ETA.
Attachment #8888782 - Attachment is obsolete: true
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.
(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
Attachment #8888785 - Flags: review?(amarchesini)
Attachment #8888786 - Flags: review?(amarchesini)
Attachment #8888785 - Flags: review?(amarchesini) → review+
Attachment #8888786 - Flags: review?(amarchesini) → review+
Attachment #8888785 - Attachment is obsolete: true
Attachment #8888786 - Attachment is obsolete: true
Pushed by email@example.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
Ah wait, doesn't look like I need to update any docs to account for this.
You need to log in before you can comment on or make changes to this bug.