third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context-expected.html - Issue 2107233002: Reland "Implement FullScreen using top layer."

Unified Diff: third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context-expected.html

Issue 2107233002: Reland "Implement FullScreen using top layer." (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: restore !important to overflow:hidden Created 4 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context.html ('k') | third_party/WebKit/LayoutTests/fullscreen/full-screen-video-has-backdrop.html » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context-expected.html

diff --git a/third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context-expected.html b/third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context-expected.html

index 98a5e31ad17a27b3e3fa01c0c8979a3b2376a036..203a33daa8e5bc956db6d471afbf0a1d253eee17 100644

--- a/third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context-expected.html

+++ b/third_party/WebKit/LayoutTests/fullscreen/full-screen-stacking-context-expected.html

@@ -1,12 +1,20 @@

<!DOCTYPE html>

-<video></video>

- var runPixelTests = true;

+var runPixelTests = true;

+function init() {

+ waitForEventAndEnd(document, 'webkitfullscreenchange');

+ runWithKeyDown(function() {

+ document.getElementById('video').webkitRequestFullScreen();

+ });

</script>

-<script>

- document.onwebkitfullscreenchange = endTest;

- runWithKeyDown(function() {

- document.querySelector("video").webkitRequestFullScreen();

- });

-</script>

+<style>

+#video {

+ background-color: black;

foolip 2016/07/06 13:19:43 This change seems to mask an actual regression in

dsinclair 2016/07/06 13:32:31 This is the expected file, so this is what the cl

foolip 2016/07/06 14:14:51 Right, but what I noticed when trying to simplify

dsinclair 2016/07/06 14:17:38 It wasn't deliberate. It sounds like there is an i

foolip 2016/07/06 15:20:27 I have a hard time deciding what would make most s

I have a hard time deciding what would make most sense, but in any case the handling of background will change from :-webkit-full-screen { background-color: white; } video:-webkit-full-screen, audio:-webkit-full-screen { background-color: transparent !important; } to just :not(:root):-webkit-full-screen::backdrop { background: black; } So in a lot of cases it will go from white to black, and this is such a case. It's just that the ::backdrop pseudo doesn't actually work for video elements, if it did then the line I've commented on wouldn't be needed. Maybe I'll push back against the spec regarding the specific colors, but in any case ::backdrop needs to work. I was right to suspect replaced content, in StyleResolver::createPseudoElementIfNeeded it bails out because !canHaveGeneratedChildren(*parentLayoutObject), which happens for media elements, some input elements and all LayoutReplaced things. It can be "fixed" by just bypassing the check for PseudoIdBackdrop, but I doubt that's sensible. I'll create a separate review for that I think.

+</style>

+<body onload="init()">

+ <video id='video'></video>

+</body>