Closed
Bug 973384
Opened 9 years ago
Closed 6 years ago
Assertion failure when <track src> is a URL that redirects
Categories
(Core :: Audio/Video: Playback, defect)
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: jruderman, Assigned: hchang)
References
Details
(Keywords: assertion, memory-leak, testcase)
Attachments
(3 files)
161 bytes,
text/html
|
Details | |
4.22 KB,
text/plain
|
Details | |
59 bytes,
text/x-review-board-request
|
rillian
:
review+
gchang
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta+
|
Details |
###!!! ASSERTION: Did not receive all required callbacks!: 'NS_FAILED(mResult) || mExpectedCallbacks == 0', file netwerk/base/src/nsAsyncRedirectVerifyHelper.cpp, line 69
Reporter | ||
Comment 1•9 years ago
|
||
Reporter | ||
Comment 2•9 years ago
|
||
This bug also causes the throbber to spin forever and makes Firefox leak nsGlobalWindow & nsDocument.
Keywords: mlk
Comment 4•9 years ago
|
||
Will do.
Updated•9 years ago
|
tracking-firefox28:
--- → ?
Comment 5•9 years ago
|
||
This blocks the tab for me, not the whole ui. I think we should wontfix 28 and patch 29 if the fix is straightforward. Redirects should be rare for track elements in the near term.
Updated•9 years ago
|
status-firefox27:
--- → unaffected
status-firefox28:
--- → affected
status-firefox29:
--- → affected
status-firefox30:
--- → affected
Comment 6•9 years ago
|
||
If this isn't a release blocker for 28 we won't track, you can nominate an uplift to Aurora when ready and we can evaluate the risk/rewards along with the timing.
Comment 7•9 years ago
|
||
We are able to work around this with some effort, but right now we (Vimeo) use redirects for our track urls, so this is kind of important for us. I've set up a test case here: https://brad.is/reportingbugs/texttracks/redirect/ Both videos properly show the captions in Chrome, Safari, and IE.
Updated•8 years ago
|
Component: Audio/Video → Audio/Video: Playback
Comment 8•6 years ago
|
||
(In reply to Lukas Blakk [:lsblakk] use ?needinfo from comment #6) > If this isn't a release blocker for 28 we won't track, you can nominate an > uplift to Aurora when ready and we can evaluate the risk/rewards along with > the timing. We just met the issue as we use a load balancer to dispatch to our subtitles assets... -- Could you continue the work on this bug ? Others browsers works well and we have to make specific cases for Firefox... which is quite a pain, and we're unable to use native HTML5 tracks. Thanks ! Vincent.
Comment 9•6 years ago
|
||
Anthony, is anyone available to look at this? It seems work died off on it years ago, but its affecting large sites on the web.
Flags: needinfo?(ajones)
Comment 10•6 years ago
|
||
Here is a unit test case: 1/ http://codepen.io/tzi/full/OWvqdr/ The VTT file is behind a 302 (cause of a load balancer) The subtitles aren't displayed in Firefox. No problem in blink browsers. 2/ http://codepen.io/tzi/full/EZEMzW/ The VTT file is directly loaded This time, the subtitles are displayed in Firefox
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8833144 -
Flags: review?(giles)
Assignee | ||
Comment 12•6 years ago
|
||
Hi Ralph, Could you please have it a review again? I am not able to carry over r+ to reviewboard. BTW, given that we actually have web-platform test for track redirection (but haven't been enabled yet), do you think if we should add our own test case at this time or we can wait for the web-platform tests enabled? (coming soon!) Thanks.
Assignee | ||
Updated•6 years ago
|
Assignee: rick.eyre → hchang
Comment 13•6 years ago
|
||
Thanks/all for the quick reply ! We definitely need this :)
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8833144 [details] Bug 973384 - Call |cb| before WebVTTListener::AsyncOnChannelRedirect returns. https://reviewboard.mozilla.org/r/109364/#review110652
Attachment #8833144 -
Flags: review?(giles) → review+
Comment 15•6 years ago
|
||
Once we have confirmation this fixes the issue cleanly we should request uplift to Aurora53, since this is an issue affecting major sites.
Updated•6 years ago
|
Flags: needinfo?(ajones)
Assignee | ||
Updated•6 years ago
|
Blocks: CVE-2017-5408
Comment hidden (mozreview-request) |
Comment 17•6 years ago
|
||
Pushed by hchang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/876e2bc7fc06 Call |cb| before WebVTTListener::AsyncOnChannelRedirect returns. r=rillian
Comment 18•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/876e2bc7fc06
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 19•6 years ago
|
||
Thank you so much for this fix! We really appreciate your responsiveness: it's pretty incredible to solve this 3 years bug in 5 days :D We would confirm the solving, once ship in the Nightly. Have a good day! Thomas.
Comment 20•6 years ago
|
||
This looks like a good candidate for backporting once verified on Nightly?
status-firefox51:
--- → wontfix
status-firefox52:
--- → affected
status-firefox53:
--- → affected
Flags: needinfo?(giles)
Comment 21•6 years ago
|
||
Thomas, this fix should be available in today's Firefox Nightly. I don't see an assertion from the test case. If you could confirm the issue is fixed for you, we'd appreciate it.
Comment 22•6 years ago
|
||
Hi Ralph, I can confirm the issue is fixed in the nightly. Thank you !
Comment 23•6 years ago
|
||
Comment on attachment 8833144 [details] Bug 973384 - Call |cb| before WebVTTListener::AsyncOnChannelRedirect returns. Nominating this for uplift to 53 and 52 to get the fix deployed more quickly. Approval Request Comment [Feature/Bug causing the regression]: WebVTT video subtitle playback. [User impact if declined]: We will not honor http redirects on subtitle files, causing some files to fail to load. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: Change is not risky for Aurora. I believe risk is acceptable for Beta. [Why is the change risky/not risky?]: This is a one-line code change, accompanied by status updates for previously failing tests. It just corrects the loading behaviour to handle redirects the way we do for other media types. I expect any regression caused by this change should be obvious, limited to the individual subtitle track, and easy to revert. [String changes made/needed]: None.
Flags: needinfo?(giles)
Attachment #8833144 -
Flags: approval-mozilla-beta?
Attachment #8833144 -
Flags: approval-mozilla-aurora?
Comment 24•6 years ago
|
||
Comment on attachment 8833144 [details] Bug 973384 - Call |cb| before WebVTTListener::AsyncOnChannelRedirect returns. Fix an assertion failure. Aurora53+.
Attachment #8833144 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 25•6 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/620b79612ae4
Comment 26•6 years ago
|
||
Comment on attachment 8833144 [details] Bug 973384 - Call |cb| before WebVTTListener::AsyncOnChannelRedirect returns. fix handling of redirects for WebVTT, beta52+
Attachment #8833144 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 27•6 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/4840d6a750cc
Comment 29•6 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/4840d6a750cc
status-firefox-esr52:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•