Skip to content

Commit

Permalink
Remove special case for <dialog> in fullscreen display:contents code
Browse files Browse the repository at this point in the history
When I added this code [1], I special-cased <dialog> and ::backdrop.
But according to the spec [2], any top layer element whose display
property is `contents` should compute to `block`. This CL makes the
full change.

This changes behavior, but in a very corner-case situation (display:
contents fullscreen elements), and Chromiums code now complies with
the spec. Gecko also agrees with the new behavior. Webkit is difficult
to test because their fullscreen impl is broken. I'm going to monitor
for bug reports in Canary/Dev/Beta and will roll this back out if any
compat issues are found.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3102297
[2] https://fullscreen.spec.whatwg.org/#new-stacking-layer

Bug: 1240701
Change-Id: Id659be8027e257758e7d8ec1b86ec87125b0cb0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749114
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1021448}
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Jul 7, 2022
1 parent fe9314d commit 8755b59
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
8 changes: 8 additions & 0 deletions fullscreen/api/fullscreen-display-contents-ref.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<title>Test that display:contents on fullscreen elements acts like display:block</title>
<meta charset="utf-8">

<div>Fullscreen Element</div>
<p>Fullscreen is display:block</p>
<p>Fullscreen::backdrop is display:block</p>
<p>After exiting fullscreen, element is display:contents</p>
49 changes: 49 additions & 0 deletions fullscreen/api/fullscreen-display-contents.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<!DOCTYPE html>
<html class="reftest-wait">
<title>Test that display:contents on fullscreen elements acts like display:block</title>
<meta charset="utf-8">
<link rel="author" href="mailto:masonf@chromium.org">
<link rel="help" href="https://fullscreen.spec.whatwg.org/#new-stacking-layer">
<link rel="match" href="fullscreen-display-contents-ref.html">
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>

<div id=target>Fullscreen Element</div>
<p>Fullscreen is display:<span id="computed-value-during">INVALID</span></p>
<p>Fullscreen::backdrop is display:<span id="computed-value-backdrop-during">INVALID</span></p>
<p>After exiting fullscreen, element is display:<span id="computed-value-after">INVALID</span></p>

<style>
#target {
display: contents;
background-color: green;
}
#target::backdrop {
display: contents;
}
</style>

<script>
target = document.querySelector("#target");
test_driver.bless("fullscreen")
.then(() => target.requestFullscreen())
.then(() => new Promise(resolve => requestAnimationFrame(resolve)))
.then(() => {
if (!target || document.fullscreenElement !== target) {
document.body.appendChild(document.createTextNode("FAIL: not fullscreen"));
} else {
document.getElementById("computed-value-during").textContent = getComputedStyle(target).display;
document.getElementById("computed-value-backdrop-during").textContent = getComputedStyle(target, "::backdrop").display;
}
})
.then(() => document.exitFullscreen())
.then(() => new Promise(resolve => requestAnimationFrame(resolve)))
.then(() => {
if (document.fullscreenElement !== null) {
document.body.appendChild(document.createTextNode("FAIL: unable to exit fullscreen"));
} else {
document.getElementById("computed-value-after").textContent = getComputedStyle(target).display;
document.documentElement.classList.remove("reftest-wait");
}
});
</script>

0 comments on commit 8755b59

Please sign in to comment.