[webkit-reviews] review denied: [Bug 28327] Media layout tests should have a way to provide test files in different formats : [Attachment 39461] Second round of changes - 20 tests

Thu Sep 17 07:30:07 PDT 2009

Eric Carlson <eric.carlson at apple.com> has denied Hin-Chung Lam
<hclam at google.com>'s request for review:
Bug 28327: Media layout tests should have a way to provide test files in
different formats
https://bugs.webkit.org/show_bug.cgi?id=28327

Attachment 39461: Second round of changes - 20 tests
https://bugs.webkit.org/attachment.cgi?id=39461&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
> --- a/LayoutTests/media/video-controls-visible-audio-only.html

> -	   <video id="no-video-media" controls loop src="content/test.wav"
onload="start()"></video>
> +	   <video id="no-video-media" controls loop></video>

> +	   <script>
> +	       setSrcById("fr", findMediaFile("audio", "content/test"));
> +	       start();
> +	   </script>

Calling start() immediately after setting the src attribute instead of
triggering it from the <video> element's 'load' event makes it extremely timing
dependent as the test assumes the movie has been loaded and parsed. You should
be able to leave the onload attribute on the <video> element even though you
set the src attribute later.

> --- a/LayoutTests/media/video-display-toggle.html

> -    <body onload="test()">
> +    <body onload="setSrcById('vid', findMediaFile('video', 'content/test'));
test()">

It would be cleaner to call setSrcById(...) from inside of the test function,
since it is already triggered on load.

> --- a/LayoutTests/media/video-document-types-expected.txt

>  
> -This tests that a standalone MPEG-4 file with 'sdsm' and 'odsm' tracks is
opened in a MediaDocument.
> +This tests that a standalone video file is opened in a MediaDocument.

This test is specific to MPEG-4 files with 'sdsm' and 'odsm' tracks. I think it
makes more sense to skip it on platforms that don't support MPEG-4 instead of
running it with another file type.