Remove <popup> "open" content attribute
This converts the usage of <popup open> to a WebIDL-only boolean
attribute. There will not be an "open" content attribute; the
visibility of the <popup> will be solely controllable through
the .show() and .hide() APIs.
In conjunction with removing the "open" attribute, this CL also
overrides the computed style for the "display" property for
popups, so that authors cannot cascade "display: block" onto a
<popup> and get it to be visible.
Bug: 1168738
Fixed: 1169784
Change-Id: Ie3d903a3a7f4e1cc0245ca819f036d00c194cbe0
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2648613
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Ionel Popescu <iopopesc@microsoft.com>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Auto-Submit: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#847863}
diff --git a/html/semantics/interactive-elements/the-popup-element/popup-element-basic.tentative.html b/html/semantics/interactive-elements/the-popup-element/popup-element-basic.tentative.html
index 41fa7d4..3be3fae 100644
--- a/html/semantics/interactive-elements/the-popup-element/popup-element-basic.tentative.html
+++ b/html/semantics/interactive-elements/the-popup-element/popup-element-basic.tentative.html
@@ -5,22 +5,42 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
-<popup id=popup>This is a popup</popup>
+<popup id=p1>This is a popup</popup>
+<popup id=p2 open>This is another popup, with an unused "open" attribute</popup>
<script>
test(function() {
assert_true(document.createElement('popup') instanceof HTMLPopupElement);
}, "popup element exists")
- test(function() {
- const popup = document.getElementById('popup');
- assert_equals(window.getComputedStyle(popup).getPropertyValue('display'),'none');
- assert_equals(popup.getAttribute('open'),null);
- popup.show();
- assert_equals(popup.getAttribute('open'),"");
- assert_equals(window.getComputedStyle(popup).getPropertyValue('display'),'block');
- popup.hide();
- assert_equals(popup.getAttribute('open'),null);
- assert_equals(window.getComputedStyle(popup).getPropertyValue('display'),'none');
- }, "popup element basic show/hide behavior")
+ const popup1 = document.getElementById('p1');
+ const popup2 = document.getElementById('p2');
+
+ test(function(){
+ assert_false(popup1.open);
+ assert_false(popup2.open);
+ assert_equals(getComputedStyle(popup2).display,"none");
+ }, 'The IDL "open" content attribute must have no effect, and must not be reflected to the WebIDL.');
+
+ test(function(){
+ assert_false(popup1.open);
+ assert_false(popup1.hasAttribute("open"));
+ assert_equals(getComputedStyle(popup1).display,"none");
+ popup1.open = true; // Should have no effect
+ assert_false(popup1.open);
+ popup1.show();
+ assert_true(popup1.open);
+ assert_false(popup1.hasAttribute("open"));
+ assert_not_equals(getComputedStyle(popup1).display,"none");
+ popup1.open = false; // Should have no effect
+ assert_true(popup1.open);
+ popup1.hide();
+ assert_false(popup1.open);
+ assert_false(popup1.hasAttribute("open"));
+ assert_equals(getComputedStyle(popup1).display,"none");
+ popup1.setAttribute("open","");
+ assert_false(popup1.open,'Setting open attribute should not reflect to the WebIDL');
+ assert_true(popup1.hasAttribute("open"));
+ assert_equals(getComputedStyle(popup1).display,"none");
+ }, "The only visibility control for <popup> is through .show() and .hide().");
</script>
diff --git a/html/semantics/interactive-elements/the-popup-element/popup-hidden-display-ref.tentative.html b/html/semantics/interactive-elements/the-popup-element/popup-hidden-display-ref.tentative.html
new file mode 100644
index 0000000..7f4ced3
--- /dev/null
+++ b/html/semantics/interactive-elements/the-popup-element/popup-hidden-display-ref.tentative.html
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author title="Mason Freed" href="mailto:masonfreed@chromium.org">
+
+No popup should be displayed here.<p>
diff --git a/html/semantics/interactive-elements/the-popup-element/popup-hidden-display.tentative.html b/html/semantics/interactive-elements/the-popup-element/popup-hidden-display.tentative.html
new file mode 100644
index 0000000..c80aecf
--- /dev/null
+++ b/html/semantics/interactive-elements/the-popup-element/popup-hidden-display.tentative.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author title="Mason Freed" href="mailto:masonfreed@chromium.org">
+<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=match href="popup-hidden-display-ref.tentative.html">
+
+No popup should be displayed here.<p>
+
+<popup>This content should be hidden</popup>
+
+<style>
+ popup {
+ display: block; /* This should have no effect */
+ top: 0;
+ left: 0;
+ width: 300px;
+ height: 200px;
+ border: 1px solid;
+ background: white;
+ color: black;
+ }
+</style>
diff --git a/html/semantics/interactive-elements/the-popup-element/popup-open-display-ref.tentative.html b/html/semantics/interactive-elements/the-popup-element/popup-open-display-ref.tentative.html
new file mode 100644
index 0000000..1b06344
--- /dev/null
+++ b/html/semantics/interactive-elements/the-popup-element/popup-open-display-ref.tentative.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author title="Mason Freed" href="mailto:masonfreed@chromium.org">
+
+<div>This is a popup</div>
+
+<style>
+ div {
+ /* Per spec: */
+ display: block;
+ position: fixed;
+ top: 0;
+ left: 0;
+ /* Per settings in test file: */
+ width: fit-content;
+ height: fit-content;
+ border: 1px solid;
+ padding: 1em;
+ background: -internal-light-dark(white, black);
+ color: -internal-light-dark(black, white);
+ }
+</style>
diff --git a/html/semantics/interactive-elements/the-popup-element/popup-open-display.tentative.html b/html/semantics/interactive-elements/the-popup-element/popup-open-display.tentative.html
new file mode 100644
index 0000000..0f6bd93
--- /dev/null
+++ b/html/semantics/interactive-elements/the-popup-element/popup-open-display.tentative.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author title="Mason Freed" href="mailto:masonfreed@chromium.org">
+<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=match href="popup-open-display-ref.tentative.html">
+
+<popup>This is a popup</popup>
+
+<style>
+ popup {
+ width: fit-content;
+ height: fit-content;
+ border: 1px solid;
+ padding: 1em;
+ background: white;
+ color: black;
+ }
+</style>
+
+<script>
+ document.querySelector('popup').show();
+</script>
diff --git a/html/semantics/interactive-elements/the-popup-element/popup-open.tentative.html b/html/semantics/interactive-elements/the-popup-element/popup-open.tentative.html
deleted file mode 100644
index d622b7d..0000000
--- a/html/semantics/interactive-elements/the-popup-element/popup-open.tentative.html
+++ /dev/null
@@ -1,29 +0,0 @@
-<!DOCTYPE html>
-<meta charset="utf-8">
-<link rel="author" title="Mason Freed" href="mailto:masonfreed@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
-<script src="/resources/testharness.js"></script>
-<script src="/resources/testharnessreport.js"></script>
-
-<popup id=p1>This is a closed popup</popup>
-<popup id=p2 open>This is an open popup</popup>
-
-<script>
- var popup1 = document.getElementById('p1');
- var popup2 = document.getElementById('p2');
-
- test(function(){
- assert_false(popup1.open);
- assert_true(popup2.open);
- }, "The IDL open attribute must return true if the content open attribute is set, and false if it is absent.");
-
- test(function(){
- assert_false(popup1.hasAttribute("open"));
- popup1.open = true;
- assert_true(popup1.hasAttribute("open"));
- popup1.open = false;
- assert_true(popup2.hasAttribute("open"));
- popup2.open = false;
- assert_false(popup2.hasAttribute("open"));
- }, "On setting, the content open attribute must be removed if the IDL open attribute is set to false, and must be present if the IDL open attribute is set to true.");
-</script>
