Closed
Bug 1389844
Opened 7 years ago
Closed 7 years ago
[MSE] Shaka Player autoplay no longer works on ifinitytv.it and mediasetpremium.it streaming services in Firefox 55
Categories
(Core :: Audio/Video: Playback, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: cpeterson, Assigned: jya)
References
Details
(Keywords: regression)
Attachments
(3 files)
59 bytes,
text/x-review-board-request
|
mozbugz
:
review+
lizzard
:
approval-mozilla-beta+
lizzard
:
approval-mozilla-esr52-
|
Details |
59 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
Shaka Player autoplay no longer works on https://www.infinitytv.it/ and https://play.mediasetpremium.it/ streaming services. STR: 1. Log into testmds5414.cloudapp.net. 2. Scroll down to "Max 2: White House Hero". 3. Hover over and click Max's play button. 4. The video screenshot pops up. 5. Click the "GUARDA DALL'INIZIO" button. RESULT: The video will buffer for a couple seconds and then either play or just keep buffering forever. I bisected this regression to BBC autoplay bug 1382303. I had a hard time reproducing this regression reliably. I could reproduce it consistently in some Firefox user profiles but not others, even for the same Firefox version.
Reporter | ||
Comment 1•7 years ago
|
||
Jean-Yves, this autoplay regression was reported in the Shaka Player's issue tracker here: https://github.com/google/shaka-player/issues/958
Flags: needinfo?(jyavenard)
Assignee | ||
Comment 2•7 years ago
|
||
I read the comment. The fix in Firefox is sound. If autoplay is active, playing event shouldn't be fired until the readyState is greater or equal than HAVE_FUTURE_DATA as per spec. The only reason those sites can fail, is because they wait on an event that won't be fired until the readyState change. It worked before because those events (play, playing) were fired incorrectly.
Flags: needinfo?(jyavenard)
Comment 3•7 years ago
|
||
Jean-Yves, was https://github.com/google/shaka-player/issues/958#issuecomment-322014131 useful? Or should we move this to Tech Evangelism?
Flags: needinfo?(jyavenard)
Assignee | ||
Comment 4•7 years ago
|
||
I am currently investigating the issue based on a comment in the github entry, that is they had 15s of data (https://github.com/google/shaka-player/issues/958#issuecomment-321926944) which should have triggered the event. However, at this stage, the new mochitest I wrote to ensure that our behaviour is indeed correct is working just fine. The mochitest loads 10.01s of data and the events are properly fired as expected.
Flags: needinfo?(jyavenard)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8897938 [details] Bug 1389844 - P1. Add mochitest. https://reviewboard.mozilla.org/r/169228/#review174652
Attachment #8897938 -
Flags: review?(gsquelart) → review+
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8897939 [details] Bug 1389844 - P2. Add Intervals::ContainsWithStrictEnd method. https://reviewboard.mozilla.org/r/169230/#review174654
Attachment #8897939 -
Flags: review?(gsquelart) → review+
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8897940 [details] Bug 1389844 - P3. Handle case where currentTime isn't contained in buffered range. https://reviewboard.mozilla.org/r/169232/#review174658
Attachment #8897940 -
Flags: review?(gsquelart) → review+
Comment 14•7 years ago
|
||
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5e67b69602da P1. Add mochitest. r=gerald https://hg.mozilla.org/integration/autoland/rev/68e2decc70c9 P2. Add Intervals::ContainsWithStrictEnd method. r=gerald https://hg.mozilla.org/integration/autoland/rev/45a51fab5dbf P3. Handle case where currentTime isn't contained in buffered range. r=gerald
Assignee | ||
Comment 15•7 years ago
|
||
[Tracking Requested - why for this release]: Prevent some site's using Google shaka's player to start playback.
tracking-firefox56:
--- → ?
tracking-firefox-esr52:
--- → ?
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5e67b69602da https://hg.mozilla.org/mozilla-central/rev/68e2decc70c9 https://hg.mozilla.org/mozilla-central/rev/45a51fab5dbf
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Comment 18•7 years ago
|
||
Comment on attachment 8897938 [details] Bug 1389844 - P1. Add mochitest. Approval Request Comment [Feature/Bug causing the regression]: bug 1382303 [User impact if declined]: Playback of some video sites will never start. [Is this code covered by automated tests?]: Yes, mochitest were written to verify coverage [Has the fix been verified in Nightly?]: Yes, original reporter communicated to me that the issue was fixed [Needs manual test from QE? If yes, steps to reproduce]: no, as there's mochitests in place [List of other uplifts needed for the feature/fix]: all patches of bug 1391666 [Is the change risky?]: no [Why is the change risky/not risky?]: We relax the behaviour for some streams whose content doesn't start exactly at 0 (which is unfortunately rather common). In the worse case we more losely say we are ready to play something when we may not, but this condition isn't fatal and will always recover. [String changes made/needed]: none [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: see above Fix Landed on Version: 57 Risk to taking this patch (and alternatives if risky): There are alternatives to make on the JS video player, but people are unlikely to do those. String or UUID changes made by this patch: none For a full fix of the problem reported, bug 1391666 is required.
Attachment #8897938 -
Flags: approval-mozilla-esr52?
Attachment #8897938 -
Flags: approval-mozilla-beta?
Comment 19•7 years ago
|
||
Comment on attachment 8897938 [details] Bug 1389844 - P1. Add mochitest. Sounds like this may affect many users, let's uplift all of this for beta 6. I think best to leave it out of ESR though as it is not a sec issue, not fixing a top crash, and we are heading into 52.4.0esr.
Attachment #8897938 -
Flags: approval-mozilla-esr52?
Attachment #8897938 -
Flags: approval-mozilla-esr52-
Attachment #8897938 -
Flags: approval-mozilla-beta?
Attachment #8897938 -
Flags: approval-mozilla-beta+
Comment 20•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/7f465d33e70a https://hg.mozilla.org/releases/mozilla-beta/rev/e33243e16a3a https://hg.mozilla.org/releases/mozilla-beta/rev/4600e5c1d853
Flags: in-testsuite+
Updated•7 years ago
|
Assignee | ||
Comment 21•7 years ago
|
||
Why the won't fix on esr52? The code that cause this regression was very recently uplifted to esr52.
Flags: needinfo?(ryanvm)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(ryanvm) → needinfo?(lhenry)
Assignee | ||
Comment 22•7 years ago
|
||
Liz, I think you should reconsider, it's a very recent regression in esr52, one introduced last month.
Comment 23•7 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #18) > [Is this code covered by automated tests?]: Yes, mochitest were written to > verify coverage > [Has the fix been verified in Nightly?]: Yes, original reporter communicated > to me that the issue was fixed > [Needs manual test from QE? If yes, steps to reproduce]: no, as there's > mochitests in place Setting qe-verify- based on jya's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Comment 24•7 years ago
|
||
Julien, what do you think? (Since you are ESR owner for this cycle)
Flags: needinfo?(lhenry) → needinfo?(jcristau)
Comment 25•7 years ago
|
||
I think I would prefer backing out the changes in bug 1382303 to taking these new ones. From https://bugzilla.mozilla.org/show_bug.cgi?id=1382303#c24 it sounds like the BBC had a workaround for that bug.
Comment 26•7 years ago
|
||
backout |
Fixed on ESR52 by backing out bug 1382303 at Julien's request. https://hg.mozilla.org/releases/mozilla-esr52/rev/f01155fe4d54
Flags: needinfo?(jcristau)
You need to log in
before you can comment on or make changes to this bug.
Description
•