Rename initiallyopen to defaultopen for popup

See the resolution below, but we've decided that `defaultopen` is a better name for this feature. No functional changes. openui/open-ui#500 Bug: 1307772 Change-Id: I9e11739a34d5180df889c9f93622bbddbd9bf982 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615796 Auto-Submit: Mason Freed <[email protected]> Reviewed-by: Joey Arhar <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Commit-Queue: Mason Freed <[email protected]> Cr-Commit-Position: refs/heads/main@{#997905} NOKEYCHECK=True GitOrigin-RevId: 9222b44127c7570b17204d5a8f90558b5cc85ad3
mjfroman · Apr 30, 2022 · 7ade694 · 7ade694
1 parent 697575c
commit 7ade694
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 69 deletions.
diff --git a/blink/renderer/core/dom/element.cc b/blink/renderer/core/dom/element.cc
@@ -2337,10 +2337,10 @@ void Element::AttributeChanged(const AttributeModificationParams& params) {
   }
 
   if (params.reason == AttributeModificationReason::kByParser &&
-      name == html_names::kInitiallyopenAttr && HasValidPopupAttribute()) {
+      name == html_names::kDefaultopenAttr && HasValidPopupAttribute()) {
     DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
     DCHECK(!isConnected());
-    GetPopupData()->setHadInitiallyOpenWhenParsed(true);
+    GetPopupData()->setHadDefaultOpenWhenParsed(true);
   }
 
   if (params.reason == AttributeModificationReason::kDirectly &&
@@ -3006,12 +3006,12 @@ Node::InsertionNotificationRequest Element::InsertedInto(
       CustomElement::TryToUpgrade(*this);
   }
 
-  if (GetPopupData() && GetPopupData()->hadInitiallyOpenWhenParsed()) {
-    // If a Popup element has the `initiallyopen` attribute upon page
+  if (GetPopupData() && GetPopupData()->hadDefaultOpenWhenParsed()) {
+    // If a Popup element has the `defaultopen` attribute upon page
     // load, and it is the *first* such popup, show it.
     DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
     DCHECK(isConnected());
-    GetPopupData()->setHadInitiallyOpenWhenParsed(false);
+    GetPopupData()->setHadDefaultOpenWhenParsed(false);
     GetDocument()
         .GetTaskRunner(TaskType::kDOMManipulation)
         ->PostTask(FROM_HERE,

diff --git a/blink/renderer/core/dom/element.idl b/blink/renderer/core/dom/element.idl
@@ -140,7 +140,7 @@ callback ScrollStateCallback = void (ScrollState scrollState);
     [MeasureAs=ElementShowPopup,RuntimeEnabled=HTMLPopupAttribute,RaisesException] void showPopup();
     [MeasureAs=ElementHidePopup,RuntimeEnabled=HTMLPopupAttribute,RaisesException] void hidePopup();
     [Unscopable,CEReactions,RuntimeEnabled=HTMLPopupAttribute,Reflect] attribute DOMString popup;
-    [CEReactions,RuntimeEnabled=HTMLPopupAttribute,Reflect] attribute boolean initiallyOpen;
+    [CEReactions,RuntimeEnabled=HTMLPopupAttribute,Reflect] attribute boolean defaultOpen;
 
     // Experimental accessibility API
     [RuntimeEnabled=ComputedAccessibilityInfo] readonly attribute DOMString? computedRole;

diff --git a/blink/renderer/core/dom/popup_data.h b/blink/renderer/core/dom/popup_data.h
@@ -21,11 +21,9 @@ class PopupData final : public GarbageCollected<PopupData> {
   bool open() const { return open_; }
   void setOpen(bool open) { open_ = open; }
 
-  bool hadInitiallyOpenWhenParsed() const {
-    return had_initiallyopen_when_parsed_;
-  }
-  void setHadInitiallyOpenWhenParsed(bool value) {
-    had_initiallyopen_when_parsed_ = value;
+  bool hadDefaultOpenWhenParsed() const { return had_defaultopen_when_parsed_; }
+  void setHadDefaultOpenWhenParsed(bool value) {
+    had_defaultopen_when_parsed_ = value;
   }
 
   PopupValueType type() const { return type_; }
@@ -59,7 +57,7 @@ class PopupData final : public GarbageCollected<PopupData> {
 
  private:
   bool open_ = false;
-  bool had_initiallyopen_when_parsed_ = false;
+  bool had_defaultopen_when_parsed_ = false;
   PopupValueType type_ = PopupValueType::kNone;
   WeakMember<Element> invoker_;
 

diff --git a/blink/renderer/core/html/html_attribute_names.json5 b/blink/renderer/core/html/html_attribute_names.json5
@@ -117,7 +117,7 @@
     "imagesrcset",
     "incremental",
     "inert",
-    "initiallyopen",
+    "defaultopen",
     "inputmode",
     "integrity",
     "is",

diff --git a/...pups/popup-initiallyopen-2.tentative.html → ...popups/popup-defaultopen-2.tentative.html b/...pups/popup-initiallyopen-2.tentative.html → ...popups/popup-defaultopen-2.tentative.html
@@ -5,9 +5,9 @@
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 
-<div popup=popup id=p1 initiallyopen>
+<div popup=popup id=p1 defaultopen>
   Outer popup
-  <div popup=popup id=p2 initiallyopen>
+  <div popup=popup id=p2 defaultopen>
     Inner popup
   </div>
 </div>
@@ -18,7 +18,7 @@
       test(function(){
         assert_true(p1.matches(':popup-open'),'The first (outermost) popup should be the one that opens in this case');
         assert_false(p2.matches(':popup-open'),'The inner popup should not be open');
-      }, "The initiallyopen attribute should cause only the first popup to open");
+      }, "The defaultopen attribute should cause only the first popup to open");
     });
   });
 </script>
diff --git a/...-initiallyopen-display-ref.tentative.html → ...up-defaultopen-display-ref.tentative.html b/...-initiallyopen-display-ref.tentative.html → ...up-defaultopen-display-ref.tentative.html
diff --git a/...opup-initiallyopen-display.tentative.html → .../popup-defaultopen-display.tentative.html b/...opup-initiallyopen-display.tentative.html → .../popup-defaultopen-display.tentative.html
@@ -2,10 +2,10 @@
 <meta charset="utf-8">
 <link rel=author href="mailto:[email protected]">
 <link rel=help href="https://open-ui.org/components/popup.research.explainer">
-<link rel=match href="popup-initiallyopen-display-ref.tentative.html">
+<link rel=match href="popup-defaultopen-display-ref.tentative.html">
 
-<div popup=popup id=p1 initiallyopen>This is a popup, which should be open upon load</div>
-<div popup=popup id=p2 initiallyopen>This is a second popup with initiallyopen, which should NOT be open upon load</div>
+<div popup=popup id=p1 defaultopen>This is a popup, which should be open upon load</div>
+<div popup=popup id=p2 defaultopen>This is a second popup with defaultopen, which should NOT be open upon load</div>
 
 <style>
   [popup] {

diff --git a/blink/web_tests/external/wpt/html/semantics/popups/popup-defaultopen.tentative.html b/blink/web_tests/external/wpt/html/semantics/popups/popup-defaultopen.tentative.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel=author href="mailto:[email protected]">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<div popup=popup id=p1 defaultopen>This is a popup, which should be open upon load</div>
+<script></script> <!-- Possibly yield the parser, just to double-check -->
+<div popup=popup id=p2 defaultopen>This is a second popup with defaultopen, which should NOT be open upon load</div>
+<div popup=hint id=p2b defaultopen>This is a hint popup with defaultopen, which should NOT be open upon load</div>
+<div popup=popup id=p3>Also not visible</div>
+
+<div popup=async id=p4 defaultopen>This is an async popup with defaultopen, which should be open upon load</div>
+<div popup=async id=p5 defaultopen>This is an async popup with defaultopen, which should be open upon load</div>
+
+<script>
+  requestAnimationFrame(() => {
+    requestAnimationFrame(() => {
+      test(function(){
+        assert_true(p1.matches(':popup-open'),'defaultopen should open the popup');
+        assert_true(p1.hasAttribute('defaultopen'));
+        assert_true(p1.defaultOpen,'defaultopen should be reflected in the IDL attribute');
+        assert_false(p2.matches(':popup-open'), 'Only the first popup with defaultopen should be open on load');
+        assert_true(p2.hasAttribute('defaultopen'),'defaultopen should be present/true, even if not opened');
+        assert_true(p2.defaultOpen,'defaultopen should be present/true, even if not opened');
+
+        assert_false(p2b.matches(':popup-open'),'Only the first popup/hint with defaultopen should be open on load');
+        assert_true(p4.matches(':popup-open'),'defaultopen should open all async popups');
+        assert_true(p5.matches(':popup-open'),'defaultopen should open all async popups');
+
+        assert_false(p3.matches(':popup-open'));
+        p3.setAttribute('defaultopen','');
+        assert_false(p3.matches(':popup-open'), 'Changing defaultopen should not affect open status');
+        assert_true(p3.hasAttribute('defaultopen'));
+        assert_true(p3.defaultOpen,'defaultopen should still reflect to IDL');
+
+        p1.removeAttribute('defaultopen');
+        assert_true(p1.matches(':popup-open'),'removing defaultopen should not close the popup');
+        assert_false(p1.hasAttribute('defaultopen'),'...but it should reflect to IDL');
+
+        p1.hidePopup();
+      }, "The defaultopen attribute should affect page load only");
+    });
+  });
+</script>
diff --git a/blink/web_tests/external/wpt/html/semantics/popups/popup-initiallyopen.tentative.html b/blink/web_tests/external/wpt/html/semantics/popups/popup-initiallyopen.tentative.html
diff --git a/blink/web_tests/webexposed/element-instance-property-listing-expected.txt b/blink/web_tests/webexposed/element-instance-property-listing-expected.txt
@@ -107,6 +107,7 @@ namespace http://www.w3.org/1999/xhtml
     property contains
     property contentEditable
     property dataset
+    property defaultOpen
     property dir
     property dispatchEvent
     property draggable
@@ -139,7 +140,6 @@ namespace http://www.w3.org/1999/xhtml
     property hidePopup
     property id
     property inert
-    property initiallyOpen
     property innerHTML
     property innerText
     property inputMode
@@ -1342,6 +1342,7 @@ namespace http://www.w3.org/2000/svg
     property computedStyleMap
     property contains
     property dataset
+    property defaultOpen
     property dispatchEvent
     property elementTiming
     property firstChild
@@ -1368,7 +1369,6 @@ namespace http://www.w3.org/2000/svg
     property hasPointerCapture
     property hidePopup
     property id
-    property initiallyOpen
     property innerHTML
     property insertAdjacentElement
     property insertAdjacentHTML

diff --git a/blink/web_tests/webexposed/global-interface-listing-expected.txt b/blink/web_tests/webexposed/global-interface-listing-expected.txt
@@ -2241,10 +2241,10 @@ interface Element : Node
     getter clientWidth
     getter computedName
     getter computedRole
+    getter defaultOpen
     getter elementTiming
     getter firstElementChild
     getter id
-    getter initiallyOpen
     getter innerHTML
     getter lastElementChild
     getter localName
@@ -2378,9 +2378,9 @@ interface Element : Node
     setter ariaVirtualContent
     setter classList
     setter className
+    setter defaultOpen
     setter elementTiming
     setter id
-    setter initiallyOpen
     setter innerHTML
     setter onbeforecopy
     setter onbeforecut