Video would not playback after seek seekbar if media.autoplay.enabled = false

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: alice0775, Assigned: rbarker)

Tracking

({regression})

41 Branch
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41+ fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

[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)
See Also: → 1173848
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: nobody → rbarker
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
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)
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 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.
(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)
(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)
<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
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;
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)
Adding a tracking flag for FF41 as seeking should play the video back at the seek'd location.
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 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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bd399cc8c6ca
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Depends on: 1178858
You need to log in before you can comment on or make changes to this bug.