Add initiallyopen attribute to the <popup> element

This add an attribute called initiallyopen to the <popup> element. When this attribute is present on page load, the first such element will be shown by default. See [1] for the conversation and Open-UI resolution. This CL also re-points the explainer links for all popup tests to the new Open-UI repo. [1] openui/open-ui#311 (comment) Bug: 1168738 Change-Id: Ib95d12234f359d503dc168e6799dd2a17b0dc18e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2832157 Reviewed-by: Ionel Popescu <iopopesc@microsoft.com> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/master@{#874037} GitOrigin-RevId: 9d0421abc7790a8e3447ae6151c87557e9081146
mjfroman · Apr 20, 2021 · 3729a07 · 3729a07
1 parent 4fde274
commit 3729a07
Show file tree

Hide file tree

Showing 20 changed files with 125 additions and 11 deletions.
diff --git a/blink/renderer/core/html/html_attribute_names.json5 b/blink/renderer/core/html/html_attribute_names.json5
@@ -110,6 +110,7 @@
     "impressionexpiry",
     "incremental",
     "inert",
+    "initiallyopen",
     "inputmode",
     "integrity",
     "is",

diff --git a/blink/renderer/core/html/html_popup_element.cc b/blink/renderer/core/html/html_popup_element.cc
@@ -21,6 +21,7 @@ namespace blink {
 HTMLPopupElement::HTMLPopupElement(Document& document)
     : HTMLElement(html_names::kPopupTag, document),
       open_(false),
+      had_initiallyopen_when_parsed_(false),
       invoker_(nullptr),
       needs_repositioning_for_select_menu_(false),
       owner_select_menu_element_(nullptr) {
@@ -77,6 +78,30 @@ void HTMLPopupElement::show() {
   MarkStyleDirty();
 }
 
+Node::InsertionNotificationRequest HTMLPopupElement::InsertedInto(
+    ContainerNode& insertion_point) {
+  HTMLElement::InsertedInto(insertion_point);
+
+  if (had_initiallyopen_when_parsed_) {
+    DCHECK(isConnected()) << "This should be being inserted by the parser";
+    // If a <popup> has the initiallyopen attribute upon page
+    // load, and it is the first such popup, show it.
+    if (!GetDocument().PopupShowing())
+      show();
+    had_initiallyopen_when_parsed_ = false;
+  }
+  return kInsertionDone;
+}
+
+void HTMLPopupElement::ParserDidSetAttributes() {
+  HTMLElement::ParserDidSetAttributes();
+
+  if (FastHasAttribute(html_names::kInitiallyopenAttr)) {
+    DCHECK(!isConnected());
+    had_initiallyopen_when_parsed_ = true;
+  }
+}
+
 void HTMLPopupElement::PushNewPopupElement(HTMLPopupElement* popup) {
   auto& stack = GetDocument().PopupElementStack();
   DCHECK(!stack.Contains(popup));

diff --git a/blink/renderer/core/html/html_popup_element.h b/blink/renderer/core/html/html_popup_element.h
@@ -51,6 +51,10 @@ class HTMLPopupElement final : public HTMLElement {
   void ScheduleHideEvent();
   void MarkStyleDirty();
 
+  Node::InsertionNotificationRequest InsertedInto(
+      ContainerNode& insertion_point) override;
+  void ParserDidSetAttributes() override;
+
   // TODO(crbug.com/1197720): The popup position should be provided by the new
   // anchored positioning scheme.
   void AdjustPopupPositionForSelectMenu(ComputedStyle&);
@@ -62,6 +66,7 @@ class HTMLPopupElement final : public HTMLElement {
   static const HTMLPopupElement* NearestOpenAncestralPopup(Node*);
 
   bool open_;
+  bool had_initiallyopen_when_parsed_;
   WeakMember<Element> invoker_;
 
   bool needs_repositioning_for_select_menu_;

diff --git a/blink/renderer/core/html/html_popup_element.idl b/blink/renderer/core/html/html_popup_element.idl
@@ -6,6 +6,7 @@
 [Exposed=Window,HTMLConstructor,RuntimeEnabled=HTMLPopupElement]
 interface HTMLPopupElement : HTMLElement {
     [CEReactions] readonly attribute boolean open;
+    [CEReactions, Reflect] attribute boolean initiallyOpen;
     [CEReactions, Measure] void show();
     [CEReactions, Measure] void hide();
 };
diff --git a/...html/semantics/interactive-elements/the-popup-element/popup-anchor-nesting.tentative.html b/...html/semantics/interactive-elements/the-popup-element/popup-anchor-nesting.tentative.html
@@ -2,7 +2,7 @@
 <meta charset="utf-8" />
 <title>Popup anchor nesting</title>
 <link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script src="/resources/testdriver.js"></script>

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
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 

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
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel=author href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <link rel=match href="popup-hidden-display-ref.tentative.html">
 
 No popup should be displayed here.<p>

diff --git a/...ics/interactive-elements/the-popup-element/popup-initiallyopen-display-ref.tentative.html b/...ics/interactive-elements/the-popup-element/popup-initiallyopen-display-ref.tentative.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author href="mailto:masonf@chromium.org">
+
+<div>This is a popup, which should be open upon load</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/...mantics/interactive-elements/the-popup-element/popup-initiallyopen-display.tentative.html b/...mantics/interactive-elements/the-popup-element/popup-initiallyopen-display.tentative.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author href="mailto:masonf@chromium.org">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
+<link rel=match href="popup-initiallyopen-display-ref.tentative.html">
+
+<popup id=p1 initiallyopen>This is a popup, which should be open upon load</popup>
+<popup id=p2 initiallyopen>This is a second popup with initiallyopen, which should NOT be open upon load</popup>
+
+<style>
+  popup {
+    width: fit-content;
+    height: fit-content;
+    border: 1px solid;
+    padding: 1em;
+    background: white;
+    color: black;
+  }
+  #p1 {
+    top:0;
+  }
+  #p2 {
+    top:100px;
+  }
+</style>
+
diff --git a/.../html/semantics/interactive-elements/the-popup-element/popup-initiallyopen.tentative.html b/.../html/semantics/interactive-elements/the-popup-element/popup-initiallyopen.tentative.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author href="mailto:masonf@chromium.org">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<popup id=p1 initiallyopen>This is a popup, which should be open upon load</popup>
+<popup id=p2 initiallyopen>This is a second popup with initiallyopen, which should NOT be open upon load</popup>
+<popup id=p3>Also not visible</popup>
+
+<script>
+  test(function(){
+    assert_true(p1.open,'initiallyopen should open the popup');
+    assert_true(p1.hasAttribute('initiallyopen'));
+    assert_true(p1.initiallyOpen,'initiallyopen should be reflected in the IDL attribute');
+    assert_false(p2.open, 'Only the first popup with initiallyopen should be open on load');
+    assert_true(p2.hasAttribute('initiallyopen'),'initiallyopen should be present/true, even if not opened');
+    assert_true(p2.initiallyOpen,'initiallyopen should be present/true, even if not opened');
+
+    assert_false(p3.open);
+    p3.setAttribute('initiallyopen','');
+    assert_false(p3.open, 'Changing initiallyopen should not affect open status');
+    assert_true(p3.hasAttribute('initiallyopen'));
+    assert_true(p3.initiallyOpen,'initiallyopen should still reflect to IDL');
+
+    p1.removeAttribute('initiallyopen');
+    assert_true(p1.open,'removing initiallyopen should not close the popup');
+    assert_false(p1.hasAttribute('initiallyopen'),'...but it should reflect to IDL');
+  }, "The initiallyopen attribute should affect page load only");
+</script>
diff --git a/...semantics/interactive-elements/the-popup-element/popup-inside-display-none.tentative.html b/...semantics/interactive-elements/the-popup-element/popup-inside-display-none.tentative.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel=author href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <link rel=match href="popup-hidden-display-ref.tentative.html">
 
 No popup should be displayed here.<p>

diff --git a/.../semantics/interactive-elements/the-popup-element/popup-invoking-attribute.tentative.html b/.../semantics/interactive-elements/the-popup-element/popup-invoking-attribute.tentative.html
@@ -2,7 +2,7 @@
 <meta charset="utf-8" />
 <title>Popup invoking attribute</title>
 <link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script src="/resources/testdriver.js"></script>

diff --git a/...ntics/interactive-elements/the-popup-element/popup-light-dismiss-on-scroll.tentative.html b/...ntics/interactive-elements/the-popup-element/popup-light-dismiss-on-scroll.tentative.html
@@ -3,7 +3,7 @@
 <meta charset="utf-8" />
 <title>Popup light dismiss on scroll</title>
 <link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 

diff --git a/.../html/semantics/interactive-elements/the-popup-element/popup-light-dismiss.tentative.html b/.../html/semantics/interactive-elements/the-popup-element/popup-light-dismiss.tentative.html
@@ -2,7 +2,7 @@
 <meta charset="utf-8" />
 <title>Popup light dismiss behavior</title>
 <link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script src="/resources/testdriver.js"></script>

diff --git a/...t/html/semantics/interactive-elements/the-popup-element/popup-open-display.tentative.html b/...t/html/semantics/interactive-elements/the-popup-element/popup-open-display.tentative.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel=author href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <link rel=match href="popup-open-display-ref.tentative.html">
 
 <popup>This is a popup</popup>

diff --git a/...mantics/interactive-elements/the-popup-element/popup-open-overflow-display.tentative.html b/...mantics/interactive-elements/the-popup-element/popup-open-overflow-display.tentative.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel=author href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <link rel=match href="popup-open-overflow-display-ref.tentative.html">
 
 <div id=container>

diff --git a/...wpt/html/semantics/interactive-elements/the-popup-element/popup-shadow-dom.tentative.html b/...wpt/html/semantics/interactive-elements/the-popup-element/popup-shadow-dom.tentative.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 

diff --git a/...l/wpt/html/semantics/interactive-elements/the-popup-element/popup-stacking.tentative.html b/...l/wpt/html/semantics/interactive-elements/the-popup-element/popup-stacking.tentative.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <meta charset="utf-8">
 <link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/Popup/explainer.md">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 

diff --git a/blink/web_tests/webexposed/element-instance-property-listing-expected.txt b/blink/web_tests/webexposed/element-instance-property-listing-expected.txt
@@ -905,6 +905,7 @@ html element picture
 html element plaintext
 html element popup
     property hide
+    property initiallyOpen
     property open
     property show
 html element pre

diff --git a/blink/web_tests/webexposed/global-interface-listing-expected.txt b/blink/web_tests/webexposed/global-interface-listing-expected.txt
@@ -4034,10 +4034,12 @@ interface HTMLPictureElement : HTMLElement
     method constructor
 interface HTMLPopupElement : HTMLElement
     attribute @@toStringTag
+    getter initiallyOpen
     getter open
     method constructor
     method hide
     method show
+    setter initiallyOpen
 interface HTMLPreElement : HTMLElement
     attribute @@toStringTag
     getter width