Closed
Bug 1173844
Opened 10 years ago
Closed 10 years ago
Video would not playback after seek seekbar if media.autoplay.enabled = false
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: alice0775, Assigned: rbarker)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
[Tracking Requested - why for this release]:
Steps To Reproduce:
1. Make sure media.autoplay.enabled = false in about:config
2. Open any video
(e.g. http://www.youtube.com/watch?v=BH0BNbwqNK8 )
3. Start Play (you may need click play button twice , I will file a bug later)
4. Seek seekbar within/without buffered range
Actual Results:
Video playback stops.
Expected Results:
Video should playback properly
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3ba889bb0741&tochange=e637319144d3
Suspect : Bug 659285
Flags: needinfo?(rbarker)
Assignee | ||
Comment 1•10 years ago
|
||
When media.autoplay.enabled = false, anytime a media element's play() function is invoked without human interaction, it will be blocked. This will break many video players that implement their own controls where the play invocation is not invoked directly by human interaction. This will particularly break youtube. I think the only way to handle this case is to allow users to white list sites they want to allow to invoke the play() call without human interaction.
Flags: needinfo?(rbarker)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → rbarker
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Summary: Video would not blayback after seek seekbar if media.autoplay.enabled = false → Video would not playback after seek seekbar if media.autoplay.enabled = false
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8621770 [details] [diff] [review]
0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.autoplay.enabled-false-15061212-4542b9e.patch
This patch is intended to stop blocking autoplay once the user has interacted with media element. My concern is that since it is GetPlayedOrSeeked that a script could seek a short distance into the media and then auto play and bypass the check. Should I add a different bool just to detect if it has only been played or is there something else I can use to detect that the user has previously interacted with the element?
Attachment #8621770 -
Flags: feedback?(cpearce)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8621770 [details] [diff] [review]
0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.autoplay.enabled-false-15061212-4542b9e.patch
After experimenting with test pages and reading through the code, I don't believe it is currently possible to bypass the block by advancing the video by setting currentTime.
Attachment #8621770 -
Flags: feedback?(cpearce) → review?(cpearce)
Comment 5•10 years ago
|
||
Comment on attachment 8621770 [details] [diff] [review]
0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.autoplay.enabled-false-15061212-4542b9e.patch
Review of attachment 8621770 [details] [diff] [review]:
-----------------------------------------------------------------
Do we also need to bail out in Seek() when the pref is set if the seek is not user initiated? Otherwise script could video.currentTime=0;video.play() to defeat this?
::: dom/html/HTMLMediaElement.cpp
@@ +2181,5 @@
> HTMLMediaElement::Play(ErrorResult& aRv)
> {
> // Prevent media element from being auto-started by a script when
> // media.autoplay.enabled=false
> + if (!GetPlayedOrSeeked() && !IsAutoplayEnabled()
Please format these as one condition per line.
Comment 6•10 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #5)
> Comment on attachment 8621770 [details] [diff] [review]
> 0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.
> autoplay.enabled-false-15061212-4542b9e.patch
>
> Review of attachment 8621770 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Do we also need to bail out in Seek() when the pref is set if the seek is
> not user initiated? Otherwise script could video.currentTime=0;video.play()
> to defeat this?
ni? for this.
Flags: needinfo?(rbarker)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #6)
> (In reply to Chris Pearce (:cpearce) from comment #5)
> > Comment on attachment 8621770 [details] [diff] [review]
> > 0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.
> > autoplay.enabled-false-15061212-4542b9e.patch
> >
> > Review of attachment 8621770 [details] [diff] [review]:
> > -----------------------------------------------------------------
> >
> > Do we also need to bail out in Seek() when the pref is set if the seek is
> > not user initiated? Otherwise script could video.currentTime=0;video.play()
> > to defeat this?
>
> ni? for this.
See comment #4 in this bug. I just blew away my build but I will post the script I tried once I have rebuilt an double checked it.
Flags: needinfo?(rbarker)
Assignee | ||
Comment 8•10 years ago
|
||
<html>
<head>
<title>Video Auto Play Bypass</title>
</head>
<body>
Script Auto Play:<br>
<video src="chromecast.mp4" id="vid"></video><br>
<div>
<button onclick="document.getElementById('vid').play()">Play</button>
<button onclick="document.getElementById('vid').pause()">Stop</button>
</div>
<script>
var v = document.getElementById("vid");
var listen = function () {
console.log("got canplay event");
v.removeEventListener("canplay", listen);
v.currentTime=0;
v.play();
};
v.addEventListener("canplay", listen);
</script>
</body>
</html>
This is the page I used to try and bypass the auto play block. It is located:
http://people.mozilla.org/~rbarker/video-bypass.html
Assignee | ||
Comment 9•10 years ago
|
||
Additionally:
<html>
<head>
<title>Video Auto Play Bypass</title>
</head>
<body>
Script Auto Play:<br>
<video src="chromecast.mp4" id="vid"></video><br>
<div>
<button onclick="document.getElementById('vid').play()">Play</button>
<button onclick="document.getElementById('vid').pause()">Stop</button>
</div>
<script>
var v = document.getElementById("vid");
v.currentTime=0;
v.play();
</script>
</body>
</html>
Will not work in firefox regardless of auto play blocking becuase:
E/GeckoConsole(15142): [JavaScript Error: "InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable" {file: "http://people.mozilla.org/~rbarker/video-bypass.html" line: 14}]
Which is referring to v.currentTime=0;
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8621770 -
Attachment is obsolete: true
Attachment #8621770 -
Flags: review?(cpearce)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8623349 [details] [diff] [review]
0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.autoplay.enabled-false-15061616-4ca7125.patch
Addressed code formatting issue.
Attachment #8623349 -
Flags: review?(cpearce)
Comment 12•10 years ago
|
||
Adding a tracking flag for FF41 as seeking should play the video back at the seek'd location.
Comment 13•10 years ago
|
||
Comment on attachment 8623349 [details] [diff] [review]
0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.autoplay.enabled-false-15061616-4ca7125.patch
Review of attachment 8623349 [details] [diff] [review]:
-----------------------------------------------------------------
Your logic fails if script calls play() in the "seeked" event handler, as by that stage GetPlayedOrSeeked() returns true, causing your logic to not block the play().
For example:
<html><head>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<title>Video Auto Play Bypass</title>
</head>
<body>
Script Auto Play:<br>
<video src="Video%20Auto%20Play%20Bypass_files/chromecast.mp4" id="vid"></video><br>
<div>
<button onclick="document.getElementById('vid').play()">Play</button>
<button onclick="document.getElementById('vid').pause()">Stop</button>
</div>
<script>
var v = document.getElementById("vid");
var name = "canplay";
var listen = function () {
console.log("got " + name + " event");
v.removeEventListener(name, listen);
v.currentTime = 0;
v.addEventListener("seeked", function() {v.play();});
};
v.addEventListener(name, listen);
</script>
</body></html>
I think you should just need to bypass the test if we've played not before, and you can quickly test that by checking whether GetPlayed() is returning a TimeRanges object with length 0.
Attachment #8623349 -
Flags: review?(cpearce) → review-
Comment 14•10 years ago
|
||
Comment on attachment 8623349 [details] [diff] [review]
0001-Bug-1173844-Video-would-not-playback-after-seek-seekbar-if-media.autoplay.enabled-false-15061616-4ca7125.patch
Review of attachment 8623349 [details] [diff] [review]:
-----------------------------------------------------------------
Tell you what, let's not go another round; r+ with the change I list below, assuming it fixes the bug for you (seems to for me).
::: dom/html/HTMLMediaElement.cpp
@@ +2181,5 @@
> HTMLMediaElement::Play(ErrorResult& aRv)
> {
> // Prevent media element from being auto-started by a script when
> // media.autoplay.enabled=false
> + if (!GetPlayedOrSeeked()
Make is:
nsRefPtr<TimeRanges> played(Played());
if (played->Length() == 0
&& !IsAutoplayEnabled()
&& !EventStateManager::IsHandlingUserInput()
&& !nsContentUtils::IsCallerChrome()) {
Attachment #8623349 -
Flags: review- → review+
Assignee | ||
Comment 15•10 years ago
|
||
carry forward r+ from :cpearce
Attachment #8623349 -
Attachment is obsolete: true
Assignee | ||
Comment 16•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 17•10 years ago
|
||
Keywords: checkin-needed
Comment 18•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•