Bug 85116

Summary: Clean up media canPlayType() layout tests
Product: WebKit Reporter: Andrew Scherkus <scherkus>
Component: MediaAssignee: Andrew Scherkus <scherkus>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch eric.carlson: review+

Andrew Scherkus
Reported 2012-04-27 18:36:20 PDT
They're a bit of mess and not all WebKit ports can take advantage of them. I'm proposing the following: * Rename video-can-play-type.html to media-can-play-type.html * Move all format-specific tests out of video-can-play-type.html into format-specific tests * Add text to each test that says it's ok if the test fails if the port does not support the format
Attachments
Patch (26.13 KB, patch)
2012-04-27 18:37 PDT, Andrew Scherkus 		eric.carlson: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Andrew Scherkus
Comment 1 2012-04-27 18:37:32 PDT
Created attachment 139313 [details] Patch
Andrew Scherkus
Comment 2 2012-04-27 18:39:10 PDT
I did other misc cleanup such as: * Add <!DOCTYPE html> * Move <script> elements from head to body * Use <body onload="start()"> on all tests * Remove unneeded CSS styling on audio/video elements I can totally understand if this makes it crazy hard to review CL and I'll revert said changes.
Eric Carlson
Comment 3 2012-04-28 14:10:33 PDT
Comment on attachment 139313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139313&action=review > LayoutTests/media/media-can-play-flac-audio.html:9 > + <script src="video-test.js"></script> > <script> Nit: why are these inside of the <body>? Here and in the rest of the tests?
Andrew Scherkus
Comment 4 2012-04-30 12:47:00 PDT
(In reply to comment #3) > (From update of attachment 139313 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=139313&action=review > > > LayoutTests/media/media-can-play-flac-audio.html:9 > > + <script src="video-test.js"></script> > > <script> > > Nit: why are these inside of the <body>? Here and in the rest of the tests? Bad habit I guess :) I'm a script-before-the-closing-body-tag kinda guy (mostly so I can access DOM elements and attach handlers right away), but considering all of these tests use the body onload handler there's no reason not to stick them in <head>
Andrew Scherkus
Comment 5 2012-05-02 00:02:06 PDT
Committed as http://trac.webkit.org/changeset/115798
Note You need to log in before you can comment on or make changes to this bug.