Bug 73583 - fullscreen/video-controls is too specific to the mac port
Summary: fullscreen/video-controls is too specific to the mac port
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-01 12:47 PST by Philippe Normand
Modified: 2012-03-06 23:46 PST (History)
3 users (show)

See Also:

Attachments
proposed patch (5.40 KB, patch)
2011-12-01 12:49 PST, Philippe Normand 		no flags Details | Formatted Diff | Diff
Patch (3.33 KB, patch)
2012-03-06 09:54 PST, Philippe Normand 		no flags Details | Formatted Diff | Diff
Patch (4.44 KB, patch)
2012-03-06 11:16 PST, Philippe Normand 		eric.carlson: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2011-12-01 12:47:24 PST 
The test on the height should be relaxed. Eric Carlson suggested to simply output the height value on the console and update the platform results.
Comment 1 Philippe Normand 2011-12-01 12:49:44 PST 
Created attachment 117462 [details]
proposed patch
Comment 2 Darin Adler 2011-12-01 13:01:13 PST 
Comment on attachment 117462 [details]
proposed patch

It’s normally important to have failure vs. success in a test. Is there anything we can do to make this a pass/fail test instead of one that logs the height?
Comment 3 Philippe Normand 2011-12-01 14:03:56 PST 
Maybe we can test height >= 0.  Would we reach concensus with this? :)
Comment 4 Philippe Normand 2012-03-06 01:45:37 PST 
(In reply to comment #3)
> Maybe we can test height >= 0.  Would we reach concensus with this? :)

I'm willing to update the test if Eric agrees with this.
Comment 5 Eric Carlson 2012-03-06 07:17:54 PST 
"height >= 0" doesn't seem especially useful, can height ever be negative? I assume that a platform will have fullscreen controls if this test is now skipped, so how about testing that height is greater than a small but reasonable value (10?, 20?)?
Comment 6 Philippe Normand 2012-03-06 07:24:29 PST 
(In reply to comment #5)
> "height >= 0" doesn't seem especially useful, can height ever be negative? I assume that a platform will have fullscreen controls if this test is now skipped, so how about testing that height is greater than a small but reasonable value (10?, 20?)?

Alright, 20 seems good enough to me. I'll update the patch!
Comment 7 Philippe Normand 2012-03-06 09:54:53 PST 
Created attachment 130399 [details]
Patch
Comment 8 WebKit Review Bot 2012-03-06 11:10:12 PST 
Comment on attachment 130399 [details]
Patch

Attachment 130399 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11833497

New failing tests:
fullscreen/video-controls-override.html
Comment 9 Philippe Normand 2012-03-06 11:16:06 PST 
Created attachment 130415 [details]
Patch
Comment 10 Philippe Normand 2012-03-06 23:46:48 PST 
Committed r110025: <http://trac.webkit.org/changeset/110025>