[webkit-reviews] review granted: [Bug 43099] Add JavaScript API to allow a page to go fullscreen : [Attachment 63477] FullScreen-LayoutTests
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 5 14:22:08 PDT 2010
Simon Fraser (smfr) <simon.fraser at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 43099: Add JavaScript API to allow a page to go fullscreen
https://bugs.webkit.org/show_bug.cgi?id=43099
Attachment 63477: FullScreen-LayoutTests
https://bugs.webkit.org/attachment.cgi?id=63477&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> Index: LayoutTests/ChangeLog
> ===================================================================
> + * fullscreen: Added.
> + * fullscreen/full-screen-api-expected.txt: Added.
> + * fullscreen/full-screen-api.html: Added.
> + * fullscreen/full-screen-css-expected.txt: Added.
> + * fullscreen/full-screen-css.html: Added.
> + * fullscreen/full-screen-request-expected.txt: Added.
> + * fullscreen/full-screen-request.html: Added.
> + * fullscreen/full-screen-test.js: Added.
> + (disableFullTestDetailsPrinting):
> + (logConsole):
> + (testAndEnd):
> + (test):
> + (testExpected):
> + (reportExpected):
> + (runSilently):
> + (run):
> + (waitForEventAndEnd):
> + (waitForEvent._eventCallback):
> + (waitForEvent):
> + (waitForEventTestAndEnd):
> + (waitForEventAndFail):
> + (waitForEventAndTest._eventCallback):
> + (waitForEventAndTest):
> + (testException):
> + (endTest):
> + (endTestLater):
> + (failTestIn):
> + (failTest):
> + (logResult):
> + (consoleWrite):
> + (relativeURL):
> + (isInTimeRanges):
You can remove the method listing from the Changelog for new files.
> Index: LayoutTests/fullscreen/full-screen-css-expected.txt
> ===================================================================
> --- LayoutTests/fullscreen/full-screen-css-expected.txt (revision 0)
> +++ LayoutTests/fullscreen/full-screen-css-expected.txt (revision 0)
> @@ -0,0 +1,13 @@
> +EXPECTED (document.defaultView.getComputedStyle(span,
null).getPropertyValue('background-color') != 'rgb(0, 255, 0)') OK
> +EXPECTED (document.defaultView.getComputedStyle(document.documentElement,
null).getPropertyValue('background-color') != 'rgb(255, 0, 0)') OK
> +EXPECTED (document.defaultView.getComputedStyle(document.documentElement,
null).getPropertyValue('color') != 'rgb(0, 0, 255)') OK
> +EVENT(webkitfullscreenchange)
> +EXPECTED (document.defaultView.getComputedStyle(span,
null).getPropertyValue('background-color') == 'rgb(0, 255, 0)') OK
> +EXPECTED (document.defaultView.getComputedStyle(document.documentElement,
null).getPropertyValue('background-color') == 'rgb(255, 0, 0)') OK
> +EXPECTED (document.defaultView.getComputedStyle(document.documentElement,
null).getPropertyValue('color') == 'rgb(0, 0, 255)') OK
> +EVENT(webkitfullscreenchange)
> +EXPECTED (document.defaultView.getComputedStyle(span,
null).getPropertyValue('background-color') == 'rgb(0, 255, 0)') OK
> +EXPECTED (document.defaultView.getComputedStyle(document.documentElement,
null).getPropertyValue('background-color') != 'rgb(255, 0, 0)'), OBSERVED
'rgb(255, 0, 0)' FAIL
You have a FAIL in the results here.
> Index: LayoutTests/fullscreen/full-screen-css.html
> ===================================================================
> +<script src=full-screen-test.js></script>
Please quote the src attribute.
> Index: LayoutTests/fullscreen/full-screen-remove-ancestor.html
> ===================================================================
> --- LayoutTests/fullscreen/full-screen-remove-ancestor.html (revision 0)
> +++ LayoutTests/fullscreen/full-screen-remove-ancestor.html (revision 0)
> @@ -0,0 +1,30 @@
> +<body>
> +<script src=full-screen-test.js></script>
> +<div><span></span></div>
> +<script>
> + var callback;
> + var fullscreenChanged = function(event)
> + {
> + if (callback)
> + callback(event)
> + };
> + waitForEvent(document, 'webkitfullscreenchange', fullscreenChanged);
> +
> + var span = document.getElementsByTagName('span')[0];
> + var div = span.parentNode;
> +
> + var spanIsFullScreen = function(event) {
> + callback = documentIsFullScreen;
> + testExpected("document.webkitCurrentFullScreenElement", span);
> + document.body.removeChild(div);
> + };
spanIsFullScreen is a confusing name for the variable; maybe
spanEnteredFullScreen.
I don't see anything actually being tested.Shouldn't you test that you came out
of fullscreen?
> Index: LayoutTests/fullscreen/full-screen-remove.html
> ===================================================================
> +<body>
> +<script src=full-screen-test.js></script>
> +<div><span></span></div>
> +<script>
> + var callback;
> + var fullscreenChanged = function(event)
> + {
> + if (callback)
> + callback(event)
> + };
> + waitForEvent(document, 'webkitfullscreenchange', fullscreenChanged);
> +
> + var span = document.getElementsByTagName('span')[0];
> +
> + var spanIsFullScreen = function(event) {
> + callback = documentIsFullScreen;
> + testExpected("document.webkitCurrentFullScreenElement", span);
> + span.parentNode.removeChild(span);
> + };
Ditto.
> Index: LayoutTests/fullscreen/full-screen-twice.html
> ===================================================================
> +<body>
> +<script src=full-screen-test.js></script>
> +<span></span>
> +<script>
> + var callback;
> + var fullscreenChanged = function(event)
> + {
> + if (callback)
> + callback(event)
> + };
> + waitForEvent(document, 'webkitfullscreenchange', fullscreenChanged);
> +
> + var span = document.getElementsByTagName('span')[0];
> +
> + var spanIsFullScreen = function() {
> + testExpected("document.webkitCurrentFullScreenElement", span);
> + callback = documentIsFullScreen;
> + document.webkitRequestFullScreen();
> + };
Same comments about it being unclear what is being tested.
If you run all the LayoutTests, you'll fine some others whose results dump all
properties of the Document object. You'll have to supply new results for these
tests, for platforms which enable the FULLSCREEN_API #ifdef.
More information about the webkit-reviews
mailing list