LayoutTests/media/video-autoplay-experiment-just-once.html - Issue 1179223002: Implement autoplay gesture override experiment.

Side by Side Diff: LayoutTests/media/video-autoplay-experiment-just-once.html

Issue 1179223002: Implement autoplay gesture override experiment. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: fixed !autoplaying behavior + test. Created 5 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 <script src=media-file.js></script>

	2 <script src=video-test.js></script>

	3 <script>

	4

	5 var mediaFile;
	philipj_slow 2015/09/04 09:24:26 It looks like none of these need to be global, if It looks like none of these need to be global, if you pass parent to prepareVideo() and move findMediaFile() into prepareVideo() liberato (no reviews please) 2015/09/08 21:58:01 Done. Show quoted text On 2015/09/04 09:24:26, philipj wrote: > It looks like none of these need to be global, if you pass parent to > prepareVideo() and move findMediaFile() into prepareVideo() Done. philipj_slow 2015/09/10 09:58:59 It doesn't matter much, but mediaFile is still glo Show quoted text On 2015/09/08 21:58:01, liberato wrote: > On 2015/09/04 09:24:26, philipj wrote: > > It looks like none of these need to be global, if you pass parent to > > prepareVideo() and move findMediaFile() into prepareVideo() > > Done. It doesn't matter much, but mediaFile is still global. liberato (no reviews please) 2015/09/10 14:52:27 Done. Show quoted text On 2015/09/10 09:58:59, philipj wrote: > On 2015/09/08 21:58:01, liberato wrote: > > On 2015/09/04 09:24:26, philipj wrote: > > > It looks like none of these need to be global, if you pass parent to > > > prepareVideo() and move findMediaFile() into prepareVideo() > > > > Done. > > It doesn't matter much, but mediaFile is still global. Done.
	6 var parent;

	7

	8 function didPlaybackStart(video)

	9 {

	10 // We say that the video started if it's not paused or if it played and

	11 // already ended.

	12 return !video.paused \|\| video.ended;
	philipj_slow 2015/09/04 09:24:26 A more direct way is to check if video.played.leng A more direct way is to check if video.played.length > 0, as nothing you do except reload can fool you into thinking that the video hasn't been played. liberato (no reviews please) 2015/09/08 21:58:01 This didn't seem to work -- it always returned fal Show quoted text On 2015/09/04 09:24:26, philipj wrote: > A more direct way is to check if video.played.length > 0, as nothing you do > except reload can fool you into thinking that the video hasn't been played. This didn't seem to work -- it always returned false. i haven't checked where that's set yet. philipj_slow 2015/09/10 09:58:59 That's surprising, but figuring out why isn't in s Show quoted text On 2015/09/08 21:58:01, liberato wrote: > On 2015/09/04 09:24:26, philipj wrote: > > A more direct way is to check if video.played.length > 0, as nothing you do > > except reload can fool you into thinking that the video hasn't been played. > > This didn't seem to work -- it always returned false. i haven't checked where > that's set yet. That's surprising, but figuring out why isn't in scope of this CL. liberato (no reviews please) 2015/09/10 14:52:27 Acknowledged. Show quoted text On 2015/09/10 09:58:59, philipj wrote: > On 2015/09/08 21:58:01, liberato wrote: > > On 2015/09/04 09:24:26, philipj wrote: > > > A more direct way is to check if video.played.length > 0, as nothing you do > > > except reload can fool you into thinking that the video hasn't been played. > > > > This didn't seem to work -- it always returned false. i haven't checked where > > that's set yet. > > That's surprising, but figuring out why isn't in scope of this CL. Acknowledged.
	13 }

	14

	15 function prepareVideo()

	16 {

	17 var video = document.createElement("video");

	18 video.src=mediaFile;
	philipj_slow 2015/09/04 09:24:26 Missing whitespace Missing whitespace liberato (no reviews please) 2015/09/08 21:58:01 Done. Show quoted text On 2015/09/04 09:24:26, philipj wrote: > Missing whitespace Done.
	19 video.setAttribute("autoplay", "true");
	philipj_slow 2015/09/04 09:24:26 video.autoplay = true works as it's a reflected at video.autoplay = true works as it's a reflected attribute. (Also, the string "true", "false" and "" all mean the same thing for boolean content attributes, so I tend to use the empty string in markup and cases like this.) liberato (no reviews please) 2015/09/08 21:58:01 Done. Show quoted text On 2015/09/04 09:24:26, philipj wrote: > video.autoplay = true works as it's a reflected attribute. (Also, the string > "true", "false" and "" all mean the same thing for boolean content attributes, > so I tend to use the empty string in markup and cases like this.) Done.
	20 parent.appendChild(video);

	21

	22 return video;

	23 }

	24

	25 function runTest()

	26 {

	27 mediaFile = findMediaFile("video", "content/test");

	28 parent = document.createElement("div");

	29 document.body.insertBefore(parent, document.body.firstChild);
	philipj_slow 2015/09/04 09:24:26 Since body has no other children (except a text no Since body has no other children (except a text node with a newline) just using appendChild would make me stop wondering if there's anything special going on here. liberato (no reviews please) 2015/09/08 21:58:01 Done. Show quoted text On 2015/09/04 09:24:26, philipj wrote: > Since body has no other children (except a text node with a newline) just using > appendChild would make me stop wondering if there's anything special going on > here. Done.
	30

	31 // Require a user gesture, but override it for muted videos.

	32 window.internals.settings.setMediaPlaybackRequiresUserGesture(true);
	philipj_slow 2015/09/04 09:24:26 Use just internals or window.internals on both lin Use just internals or window.internals on both lines. liberato (no reviews please) 2015/09/08 21:58:01 no idea. done. Show quoted text On 2015/09/04 09:24:26, philipj wrote: > Use just internals or window.internals on both lines. no idea. done.
	33 internals.settings.setAutoplayExperimentMode("enabled-forvideo-ifmuted");

	34

	35 var videoShouldPlay = prepareVideo();

	36 var videoShouldNotPlay = prepareVideo();

	37

	38 // Pause() will clear the autoplaying flag, which should also prevent the

	39 // gesture override experiment from autoplaying.

	40 videoShouldNotPlay.pause();

	41

	42 // Mute them both, and see if only one starts.

	43 videoShouldPlay.muted = true;

	44 videoShouldNotPlay.muted = true;

	45 logResult(didPlaybackStart(videoShouldPlay), "First video should play");

	46 logResult(!didPlaybackStart(videoShouldNotPlay), "Second video should not pl ay");

	47 testRunner.notifyDone();

	48 }

	49

	50 </script>

	51 <p>Test that the autoplay experiment doesn't play media once the media

	52 is no longer eligible for autoplay.</p>

	53 <body onLoad="runTest()">

	54 </body>

OLD	NEW

« no previous file with comments | « no previous file | LayoutTests/media/video-autoplay-experiment-just-once-expected.txt » ('j') | LayoutTests/media/video-autoplay-experiment-modes.html » ('J')