Review of Google's audio/video tests

I have reviewed:

http://test.w3.org/html/tests/submission/Google/audio/
http://test.w3.org/html/tests/submission/Google/video/

Below is selections of the diff of the checkin that added the tests.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/error/error_onerror_called_on_bogus_source.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/error/error_onerror_called_on_bogus_source.htmlWed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,27 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.error - MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-error">spec  
> reference</a></p>
> +  <audio id="a" src="." onerror="test_error()">
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("audioElement.onerror is triggered on bogus source");
> +function test_error() {
> + var v = document.getElementById("a");
> + t.step(function() {
> +  assert_equals(
> +   v.error.code,
> +   4,
> +   "audioElement.error MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4");
> + });
> + t.done();
> +}
> +  </script>
> + </body>
> +</html>

There's a race condition in this test. onerror might fire before the  
<script> element is parsed. Put the script before the <audio>.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/error/error_src_not_supported.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/error/error_src_not_supported.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,25 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.error - MEDIA_ERR_SRC_NOT_SUPPORTED on bogus  
> source</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-error">spec  
> reference</a></p>
> +  <audio id="a" src="." onerror="test_error()">
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +function test_error() {
> + var v = document.getElementById("a");
> + test(function() {
> +  assert_equals(
> +   v.error.code,
> +   v.error.MEDIA_ERR_SRC_NOT_SUPPORTED,
> +   "audioElement.error property is MEDIA_ERR_SRC_NOT_SUPPORTED on bogus  
> source");
> +});
> +}
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/error/error_src_not_supported_is_4.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/error/error_src_not_supported_is_4.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,25 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.error - MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-error">spec  
> reference</a></p>
> +  <audio id="a" src="." onerror="test_error()">
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +function test_error() {
> + var v = document.getElementById("a");
> + test(function() {
> +  assert_equals(
> +   v.error.code,
> +   4,
> +   "audioElement.error MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4");
> +});
> +}
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/events/event_timeupdate.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/submission/Google/audio/events/event_timeupdate.html Wed Nov  
> 17 15:11:00 2010 -0500
> @@ -0,0 +1,27 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video events - timeupdate</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#mediaevents">spec  
> reference</a></p>
> +  <video id="a" autoplay controls>
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("setting src attribute on a sufficiently long  
> autoplay video should trigger timeupdate event", {timeout:60000});
> +var a = document.getElementById("a");
> +a.addEventListener("timeupdate", function() {
> +  t.step(function() {
> +   assert_true(true);
> +  });
> +  t.done();
> +  a.pause();
> +});
> +a.src = getAudioURI("http://media.w3.org/2010/05/sound/sound_5") + "?"  
> + new Date() + Math.random();
> +  </script>
> + </body>
> +</html>

timeupdate should fire at least once regardless of the length of the video.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/events/event_timeupdate_manual.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/events/event_timeupdate_manual.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,28 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video events - timeupdate</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#mediaevents">spec  
> reference</a></p>
> +  <video id="a" controls>
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("calling play() on a sufficiently long video should  
> trigger timeupdate event", {timeout:30000});
> +var a = document.getElementById("a");
> +a.addEventListener("timeupdate", function() {
> +  t.step(function() {
> +   assert_true(true);
> +  });
> +  t.done();
> +  a.pause();
> +});
> +a.src = getAudioURI("http://media.w3.org/2010/05/sound/sound_5") + "?"  
> + new Date() + Math.random();
> +a.play();
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/networkState/networkState_during_loadstart.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/networkState/networkState_during_loadstart.htmlWed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,28 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.networkState - NETWORK_LOADING</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-networkstate">spec  
> reference</a></p>
> +  <audio id="a" autoplay controls>
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("audioElement.networkState should be NETWORK_LOADING  
> during loadstart event", {timeout:30000});
> +var a = document.getElementById("a");
> +a.addEventListener("loadstart", function() {
> +  t.step(function() {
> +   assert_equals(a.networkState,
> +   a.NETWORK_LOADING);
> +  });
> +  t.done();
> +  a.pause();
> +});
> +a.src = getAudioURI("http://media.w3.org/2010/05/sound/sound_5") + "?"  
> + new Date() + Math.random();
> +  </script>
> + </body>
> +</html>

This test is wrong. loadstart is queued on the event loop. networkState  
can change before the loadstart event is actually fired. You should test  
that it is NETWORK_LOADING or NETWORK_IDLE. If you want to reliably test  
that it doesn't change from NETWORK_LOADING before the event loop spins  
you need to throttle the server.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/networkState/networkState_during_progress.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/networkState/networkState_during_progress.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,28 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.networkState - NETWORK_LOADING</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-networkstate">spec  
> reference</a></p>
> +  <audio id="a" autoplay controls>
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("audioElement.networkState should be NETWORK_LOADING  
> during progress event", {timeout:30000});
> +var a = document.getElementById("a");
> +a.addEventListener("progress", function() {
> +  t.step(function() {
> +   assert_equals(a.networkState,
> +   a.NETWORK_LOADING);
> +  });
> +  t.done();
> +  a.pause();
> +});
> +a.src = getAudioURI("http://media.w3.org/2010/05/sound/sound_5") + "?"  
> + new Date() + Math.random();
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/preload/preload_reflects_bogus_value.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/preload/preload_reflects_bogus_value.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,22 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.preload - reflection test</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-preload">spec  
> reference</a></p>
> +  <audio id="audio" preload="marks-fantasmagorical-preload-state">
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + assert_equals(
> +  document.getElementById("audio").preload,
> +  "auto",
> +  "audioElement.preload reflects bogus value as 'auto'");
> +});
> +  </script>
> + </body>
> +</html>

Bogus value should map to the missing value default, which is UA-defined  
(but 'metadata' is suggested).

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/preload/preload_reflects_empty.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/preload/preload_reflects_empty.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,22 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.preload - reflection test</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-preload">spec  
> reference</a></p>
> +  <audio id="audio" preload="">
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + assert_equals(
> +  document.getElementById("audio").preload,
> +  "auto",
> +  "audioElement.preload reflects empty value as 'auto'");
> +});
> +  </script>
> + </body>
> +</html>

I don't find in the spec where it says that empty string should be  
reflected as 'auto'. Both empty string and 'auto' are valid keywords, so  
AFAICT preload="auto" should be reflected as 'auto' and preload="" should  
be reflected as ''.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/audio/preload/preload_reflects_no_value.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/audio/preload/preload_reflects_no_value.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,22 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>audio.preload - reflection test</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-preload">spec  
> reference</a></p>
> +  <audio id="audio">
> +  </audio>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + assert_equals(
> +  document.getElementById("audio").preload,
> +  "auto",
> +  "audioElement.preload absence reflects 'none' value");
> +});
> +  </script>
> + </body>
> +</html>

Missing attribute should map to the missing value default, which is  
UA-defined (but 'metadata' is suggested). (Also typo? 'none')

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/canPlayType/canPlayType_two_implies_one_5.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/canPlayType/canPlayType_two_implies_one_5.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,25 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.canPlayType() - codecs parameter logic</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#mime-types">spec  
> reference</a></p>
> +  <video id="v">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + var v = document.getElementById("v");
> + if (v.canPlayType('video/mp4; codecs="avc1.42E01E, mp4a.40.2"') ==  
> "probably") {
> +  assert_equals(
> +   v.canPlayType('video/mp4; codecs="avc1.42E01E, mp4a.40.2"'),
> +   v.canPlayType('video/ogg; codecs="avc1.42E01E"'),
> +   "ability to play two codecs implies the ability to play one");
> + }
> +});
> +  </script>
> + </body>
> +</html>

s/ogg/mp4/

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/canPlayType/canPlayType_two_implies_one_6.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/canPlayType/canPlayType_two_implies_one_6.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,25 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.canPlayType() - codecs parameter logic</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#mime-types">spec  
> reference</a></p>
> +  <video id="v">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + var v = document.getElementById("v");
> + if (v.canPlayType('video/mp4; codecs="avc1.42E01E, mp4a.40.2"') ==  
> "probably") {
> +  assert_equals(
> +   v.canPlayType('video/mp4; codecs="avc1.42E01E, mp4a.40.2"'),
> +   v.canPlayType('video/ogg; codecs="mp4a.40.2"'),
> +   "ability to play two codecs implies the ability to play one");
> + }
> +});
> +  </script>
> + </body>
> +</html>

s/ogg/mp4/

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/error/error_onerror_called_on_bogus_source.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/error/error_onerror_called_on_bogus_source.htmlWed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,27 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.error - MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-error">spec  
> reference</a></p>
> +  <video id="v" src="." onerror="test_error()">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("videoElement.onerror is triggered on bogus source");
> +function test_error() {
> + var v = document.getElementById("v");
> + t.step(function() {
> +  assert_equals(
> +   v.error.code,
> +   4,
> +   "videoElement.error MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4");
> + });
> + t.done();
> +}
> +  </script>
> + </body>
> +</html>

Race condition.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/error/error_src_not_supported.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/error/error_src_not_supported.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,25 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.error - MEDIA_ERR_SRC_NOT_SUPPORTED on bogus  
> source</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-error">spec  
> reference</a></p>
> +  <video id="v" src="." onerror="test_error()">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +function test_error() {
> + var v = document.getElementById("v");
> + test(function() {
> +  assert_equals(
> +   v.error.code,
> +   v.error.MEDIA_ERR_SRC_NOT_SUPPORTED,
> +   "videoElement.error property is MEDIA_ERR_SRC_NOT_SUPPORTED on bogus  
> source");
> +});
> +}
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/error/error_src_not_supported_is_4.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/error/error_src_not_supported_is_4.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,25 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.error - MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-error">spec  
> reference</a></p>
> +  <video id="v" src="." onerror="test_error()">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +function test_error() {
> + var v = document.getElementById("v");
> + test(function() {
> +  assert_equals(
> +   v.error.code,
> +   4,
> +   "videoElement.error MEDIA_ERR_SRC_NOT_SUPPORTED constant is 4");
> +});
> +}
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/events/event_timeupdate.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/submission/Google/video/events/event_timeupdate.html Wed Nov  
> 17 15:11:00 2010 -0500
> @@ -0,0 +1,27 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video events - timeupdate</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#mediaevents">spec  
> reference</a></p>
> +  <video id="v" autoplay controls>
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("setting src attribute on a sufficiently long  
> autoplay video should trigger timeupdate event", {timeout:60000});
> +var v = document.getElementById("v");
> +v.addEventListener("timeupdate", function() {
> +  t.step(function() {
> +   assert_true(true);
> +  });
> +  t.done();
> +  v.pause();
> +});
> +v.src = getVideoURI("http://media.w3.org/2010/05/video/movie_300") +  
> "?" + new Date() + Math.random();
> +  </script>
> + </body>
> +</html>

timeupdate should fire at least once regardless of the length of the video.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/events/event_timeupdate_manual.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/events/event_timeupdate_manual.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,28 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video events - timeupdate</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#mediaevents">spec  
> reference</a></p>
> +  <video id="v" controls>
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("calling play() on a sufficiently long video should  
> trigger timeupdate event", {timeout:30000});
> +var v = document.getElementById("v");
> +v.addEventListener("timeupdate", function() {
> +  t.step(function() {
> +   assert_true(true);
> +  });
> +  t.done();
> +  v.pause();
> +});
> +v.src = getVideoURI("http://media.w3.org/2010/05/video/movie_300") +  
> "?" + new Date() + Math.random();
> +v.play();
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/networkState/networkState_during_loadstart.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/networkState/networkState_during_loadstart.htmlWed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,28 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.networkState - NETWORK_LOADING</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-networkstate">spec  
> reference</a></p>
> +  <video id="v" autoplay controls>
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("videoElement.networkState should be NETWORK_LOADING  
> during loadstart event", {timeout:30000});
> +var v = document.getElementById("v");
> +v.addEventListener("loadstart", function() {
> +  t.step(function() {
> +   assert_equals(v.networkState,
> +   v.NETWORK_LOADING);
> +  });
> +  t.done();
> +  v.pause();
> +});
> +v.src = getVideoURI("http://media.w3.org/2010/05/video/movie_300") +  
> "?" + new Date() + Math.random();
> +  </script>
> + </body>
> +</html>

This test is wrong. loadstart is queued on the event loop. networkState  
can change before the loadstart event is actually fired. You should test  
that it is NETWORK_LOADING or NETWORK_IDLE. If you want to reliably test  
that it doesn't change from NETWORK_LOADING before the event loop spins  
you need to throttle the server.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/networkState/networkState_during_progress.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/networkState/networkState_during_progress.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,28 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.networkState - NETWORK_LOADING</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> +  <script src="../../../Microsoft/common/media.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-networkstate">spec  
> reference</a></p>
> +  <video id="v" autoplay controls>
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +var t = async_test("videoElement.networkState should be NETWORK_LOADING  
> during progress event", {timeout:30000});
> +var v = document.getElementById("v");
> +v.addEventListener("progress", function() {
> +  t.step(function() {
> +   assert_equals(v.networkState,
> +   v.NETWORK_LOADING);
> +  });
> +  t.done();
> +  v.pause();
> +});
> +v.src = getVideoURI("http://media.w3.org/2010/05/video/movie_300") +  
> "?" + new Date() + Math.random();
> +  </script>
> + </body>
> +</html>

ditto

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/preload/preload_reflects_bogus_value.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/preload/preload_reflects_bogus_value.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,22 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.preload - reflection test</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-preload">spec  
> reference</a></p>
> +  <video id="video" preload="marks-fantasmagorical-preload-state">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + assert_equals(
> +  document.getElementById("video").preload,
> +  "auto",
> +  "videoElement.preload reflects bogus value as 'auto'");
> +});
> +  </script>
> + </body>
> +</html>

Bogus value should map to the missing value default, which is UA-defined  
(but 'metadata' is suggested).

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/preload/preload_reflects_empty.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/preload/preload_reflects_empty.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,22 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.preload - reflection test</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-preload">spec  
> reference</a></p>
> +  <video id="video" preload="">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + assert_equals(
> +  document.getElementById("video").preload,
> +  "auto",
> +  "videoElement.preload reflects empty value as 'auto'");
> +});
> +  </script>
> + </body>
> +</html>

I don't find in the spec where it says that empty string should be  
reflected as 'auto'. Both empty string and 'auto' are valid keywords, so  
AFAICT preload="auto" should be reflected as 'auto' and preload="" should  
be reflected as ''.

> diff -r 41d1c4473fcc -r 5c9ec34d3297  
> tests/submission/Google/video/preload/preload_reflects_no_value.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++  
> b/tests/submission/Google/video/preload/preload_reflects_no_value.html Wed  
> Nov 17 15:11:00 2010 -0500
> @@ -0,0 +1,22 @@
> +<!doctype html>
> +<html>
> + <head>
> +  <title>video.preload - reflection test</title>
> +  <script src="../../../../resources/testharness.js"></script>
> +  <script src="../../../../resources/testharnessreport.js"></script>
> + </head>
> + <body>
> +  <p><a  
> href="http://dev.w3.org/html5/spec/Overview.html#dom-media-preload">spec  
> reference</a></p>
> +  <video id="video">
> +  </video>
> +  <div id="log"></div>
> +  <script>
> +test(function() {
> + assert_equals(
> +  document.getElementById("video").preload,
> +  "auto",
> +  "videoElement.preload absence reflects 'none' value");
> +});
> +  </script>
> + </body>
> +</html>

Missing attribute should map to the missing value default, which is  
UA-defined (but 'metadata' is suggested). (Also typo? 'none')

Some tests in submission/Google/audio test video. They should be changed  
to audio.

Tests that use addEventListener seem to generally lack the third argument,  
which make them not run in Opera and Firefox.

Why are there _manual.html tests? Aren't they just dups of  
non-_manual.html tests?

Other tests in submission/Google/audio and submission/Google/video that  
are not covered by the comments above are OK.

-- 
Simon Pieters
Opera Software

Received on Wednesday, 2 March 2011 12:23:34 UTC