Rename show->popupshow, hide->popuphide, and add global handlers

Per [1], there's feedback that "show" and "hide" are too generic
and could be confusing. Developers might think anything that shows
or hides (e.g. via display:none) should fire these events. So this
CL renames them to "popupshow" and "popuphide". It also adds global
event handlers, behind the flag.

[1] openui/open-ui#607 (comment)

Bug: 1307772
Change-Id: Iaec1306176e7e08dc367c25b801d6b21f19a1f88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938538
Reviewed-by: Joey Arhar <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1056698}
NOKEYCHECK=True
GitOrigin-RevId: 56587dc14d9787442b3ec53dda4e7f364a62aaeb

blink/renderer/core/dom/element.cc

@@ -2468,14 +2468,14 @@ void Element::showPopUp(ExceptionState& exception_state) {
         "Invalid on already-showing or disconnected popup elements");
   }
 
-  // Fire the show event (bubbles, cancelable).
-  Event* event = Event::CreateCancelableBubble(event_type_names::kShow);
+  // Fire the popupshow event (bubbles, cancelable).
+  Event* event = Event::CreateCancelableBubble(event_type_names::kPopupshow);
   event->SetTarget(this);
   if (DispatchEvent(*event) != DispatchEventResult::kNotCanceled)
     return;
 
-  // The 'show' event handler could have changed this pop-up, e.g. by changing
-  // its type, removing it from the document, or calling showPopUp().
+  // The 'popupshow' event handler could have changed this pop-up, e.g. by
+  // changing its type, removing it from the document, or calling showPopUp().
   if (!HasPopupAttribute() || !isConnected() || popupOpen())
     return;
 
@@ -2510,8 +2510,8 @@ void Element::showPopUp(ExceptionState& exception_state) {
                          HidePopupIndependence::kHideUnrelated);
     }
 
-    // The 'hide' event handlers could have changed this popup, e.g. by changing
-    // its type, removing it from the document, or calling showPopUp().
+    // The 'popuphide' event handlers could have changed this popup, e.g. by
+    // changing its type, removing it from the document, or calling showPopUp().
     if (!HasPopupAttribute() || !isConnected() || popupOpen())
       return;
 
@@ -2646,7 +2646,7 @@ void Element::hidePopUp(ExceptionState& exception_state) {
 // 1. Capture any already-running animations via getAnimations(), including
 //    animations on descendant elements.
 // 2. Remove the `:open` pseudo class.
-// 3. Fire the 'hide' event.
+// 3. Fire the 'popuphide' event.
 // 4. If the hidePopup() call is *not* the result of the pop-up being "forced
 //    out" of the top layer, e.g. by a modal dialog or fullscreen element:
 //   a. Restore focus to the previously-focused element.
@@ -2667,8 +2667,8 @@ void Element::HidePopUpInternal(HidePopupFocusBehavior focus_behavior,
     HideAllPopupsUntil(this, document, focus_behavior, forcing_level,
                        HidePopupIndependence::kLeaveUnrelated);
 
-    // The 'hide' event handlers could have changed this popup, e.g. by changing
-    // its type, removing it from the document, or calling hidePopUp().
+    // The 'popuphide' event handlers could have changed this popup, e.g. by
+    // changing its type, removing it from the document, or calling hidePopUp().
     if (!HasPopupAttribute() || !isConnected() ||
         GetPopupData()->visibilityState() != PopupVisibilityState::kShowing) {
       DCHECK(!GetDocument().PopupStack().Contains(this));
@@ -2702,8 +2702,8 @@ void Element::HidePopUpInternal(HidePopupFocusBehavior focus_behavior,
   GetPopupData()->setVisibilityState(PopupVisibilityState::kTransitioning);
   PseudoStateChanged(CSSSelector::kPseudoOpen);
 
-  // Fire the hide event (bubbles, not cancelable).
-  Event* event = Event::CreateBubble(event_type_names::kHide);
+  // Fire the popuphide event (bubbles, not cancelable).
+  Event* event = Event::CreateBubble(event_type_names::kPopuphide);
   event->SetTarget(this);
   if (force_hide) {
     // We will be force-hidden when the pop-up element is being removed from
@@ -2715,8 +2715,8 @@ void Element::HidePopUpInternal(HidePopupFocusBehavior focus_behavior,
   auto result = DispatchEvent(*event);
   DCHECK_EQ(result, DispatchEventResult::kNotCanceled);
 
-  // The 'hide' event handler could have changed this popup, e.g. by changing
-  // its type, removing it from the document, or calling showPopUp().
+  // The 'popuphide' event handler could have changed this popup, e.g. by
+  // changing its type, removing it from the document, or calling showPopUp().
   if (!isConnected() || !HasPopupAttribute() ||
       GetPopupData()->visibilityState() !=
           PopupVisibilityState::kTransitioning) {

blink/renderer/core/dom/global_event_handlers.h

@@ -107,6 +107,8 @@ class GlobalEventHandlers {
   DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(pointerover, kPointerover)
   DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(pointerrawupdate, kPointerrawupdate)
   DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(pointerup, kPointerup)
+  DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(popuphide, kPopuphide)
+  DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(popupshow, kPopupshow)
   DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(progress, kProgress)
   DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(ratechange, kRatechange)
   DEFINE_STATIC_ATTRIBUTE_EVENT_LISTENER(reset, kReset)

blink/renderer/core/dom/global_event_handlers.idl

@@ -83,6 +83,8 @@
     attribute EventHandler onpause;
     attribute EventHandler onplay;
     attribute EventHandler onplaying;
+    [RuntimeEnabled=HTMLPopupAttribute] attribute EventHandler onpopuphide;
+    [RuntimeEnabled=HTMLPopupAttribute] attribute EventHandler onpopupshow;
     attribute EventHandler onprogress;
     attribute EventHandler onratechange;
     attribute EventHandler onreset;

blink/renderer/core/events/event_type_names.json5

@@ -153,7 +153,6 @@
     "gestureflingstart",
     "gotpointercapture",
     "hashchange",
-    "hide",
     "icecandidate",
     "icecandidateerror",
     "iceconnectionstatechange",
@@ -232,6 +231,8 @@
     "pointerrawupdate",
     "pointerup",
     "popstate",
+    "popuphide",
+    "popupshow",
     "portalactivate",
     "prerenderingchange",
     "prioritychange",

blink/renderer/core/html/html_attribute_names.json5

@@ -239,6 +239,8 @@
     "onpointerrawupdate",
     "onpointerup",
     "onpopstate",
+    "onpopuphide",
+    "onpopupshow",
     "onportalactivate",
     "onprogress",
     "onratechange",

blink/renderer/core/html/html_element.cc

@@ -553,6 +553,10 @@ AttributeTriggers* HTMLElement::TriggersForAttributeName(
        event_type_names::kPointerup, nullptr},
       {html_names::kOnprogressAttr, kNoWebFeature, event_type_names::kProgress,
        nullptr},
+      {html_names::kOnpopuphideAttr, kNoWebFeature,
+       event_type_names::kPopuphide, nullptr},
+      {html_names::kOnpopupshowAttr, kNoWebFeature,
+       event_type_names::kPopupshow, nullptr},
       {html_names::kOnratechangeAttr, kNoWebFeature,
        event_type_names::kRatechange, nullptr},
       {html_names::kOnresetAttr, kNoWebFeature, event_type_names::kReset,

blink/web_tests/external/wpt/html/semantics/popups/popup-animation-corner-cases.tentative.html

@@ -94,7 +94,7 @@
   assert_true(isElementVisible(popUp));
   assert_equals(popUp.getAnimations({subtree: true}).length,0);
   let animation;
-  popUp.addEventListener('hide', () => {
+  popUp.addEventListener('popuphide', () => {
     animation = popUp.animate([{opacity: 1},{opacity: 0}],1000000);
   });
   assert_equals(popUp.getAnimations({subtree: true}).length,0,'There should be no animations yet');
@@ -105,7 +105,7 @@
   animation.finish();
   await waitForRender();
   assert_false(isElementVisible(popUp),'Once the animation ends, the popup is hidden');
-},'It should be possible to use the "hide" event handler to animate the hide');
+},'It should be possible to use the "popuphide" event handler to animate the hide');
 
 
 promise_test(async (t) => {
@@ -115,21 +115,21 @@
   popUp.showPopUp();
   assert_true(isElementVisible(popUp));
   assert_equals(popUp.getAnimations({subtree: true}).length,0);
-  popUp.addEventListener('hide', () => {
+  popUp.addEventListener('popuphide', () => {
     popUp.animate([{opacity: 1},{opacity: 0}],1000000);
   });
   assert_equals(popUp.getAnimations({subtree: true}).length,0,'There should be no animations yet');
   dialog.showModal(); // Force hide the popup
   await waitForRender();
   assert_equals(popUp.getAnimations({subtree: true}).length,1,'the hide animation should now be running');
   assert_false(isElementVisible(popUp),'But the animation should *not* keep the popup visible in this case');
-},'It should *not* be possible to use the "hide" event handler to animate the hide, if the hide is due to dialog.showModal');
+},'It should *not* be possible to use the "popuphide" event handler to animate the hide, if the hide is due to dialog.showModal');
 
 promise_test(async (t) => {
   const {popUp, descendent} = createPopUp(t,'');
   popUp.showPopUp();
   assert_true(isElementVisible(popUp));
-  popUp.addEventListener('hide', (e) => {
+  popUp.addEventListener('popuphide', (e) => {
     e.preventDefault();
   });
   popUp.hidePopUp();

blink/web_tests/external/wpt/html/semantics/popups/popup-events.tentative.html

@@ -11,53 +11,68 @@
 
 <script>
 window.onload = () => {
-  promise_test(async t => {
-    const popup = document.querySelector('[popup]');
-    let showCount = 0;
-    let hideCount = 0;
-    assert_false(popup.matches(':open'));
-    const controller = new AbortController();
-    const signal = controller.signal;
-    t.add_cleanup(() => controller.abort());
-    document.addEventListener('show',() => ++showCount, {signal});
-    document.addEventListener('hide',() => ++hideCount, {signal});
-    assert_equals(0,showCount);
-    assert_equals(0,hideCount);
-    popup.showPopUp();
-    assert_true(popup.matches(':open'));
-    assert_equals(1,showCount);
-    assert_equals(0,hideCount);
-    await waitForRender();
-    assert_true(popup.matches(':open'));
-    popup.hidePopUp();
-    assert_false(popup.matches(':open'));
-    assert_equals(1,showCount);
-    assert_equals(1,hideCount);
-    await waitForRender();
-    // No additional events after animation frame
-    assert_false(popup.matches(':open'));
-    assert_equals(1,showCount);
-    assert_equals(1,hideCount);
-  }, 'Show and hide events get properly dispatched for popups');
+  for(const method of ["listener","attribute"]) {
+    promise_test(async t => {
+      const popup = document.querySelector('[popup]');
+      assert_false(popup.matches(':open'));
+      let showCount = 0;
+      let hideCount = 0;
+      switch (method) {
+        case "listener":
+          const controller = new AbortController();
+          const signal = controller.signal;
+          t.add_cleanup(() => controller.abort());
+          document.addEventListener('popupshow',() => ++showCount, {signal});
+          document.addEventListener('popuphide',() => ++hideCount, {signal});
+          break;
+        case "attribute":
+          assert_false(popup.hasAttribute('onpopupshow'));
+          assert_false(popup.hasAttribute('onpopuphide'));
+          t.add_cleanup(() => popup.removeAttribute('onpopupshow'));
+          t.add_cleanup(() => popup.removeAttribute('onpopuphide'));
+          popup.onpopupshow = () => ++showCount;
+          popup.onpopuphide = () => ++hideCount;
+          break;
+        default: assert_unreached();
+      }
+      assert_equals(0,showCount);
+      assert_equals(0,hideCount);
+      popup.showPopUp();
+      assert_true(popup.matches(':open'));
+      assert_equals(1,showCount);
+      assert_equals(0,hideCount);
+      await waitForRender();
+      assert_true(popup.matches(':open'));
+      popup.hidePopUp();
+      assert_false(popup.matches(':open'));
+      assert_equals(1,showCount);
+      assert_equals(1,hideCount);
+      await waitForRender();
+      // No additional events after animation frame
+      assert_false(popup.matches(':open'));
+      assert_equals(1,showCount);
+      assert_equals(1,hideCount);
+    }, `Popupshow and popuphide events (${method}) get properly dispatched for popups`);
+  }
 
   promise_test(async t => {
     const popUp = document.querySelector('[popup]');
     const controller = new AbortController();
     const signal = controller.signal;
     t.add_cleanup(() => controller.abort());
     let cancel = true;
-    popUp.addEventListener('show',(e) => {
+    popUp.addEventListener('popupshow',(e) => {
       if (cancel)
         e.preventDefault();
     }, {signal});
     assert_false(popUp.matches(':open'));
     popUp.showPopUp();
-    assert_false(popUp.matches(':open'),'The "show" event should be cancelable');
+    assert_false(popUp.matches(':open'),'The "popupshow" event should be cancelable');
     cancel = false;
     popUp.showPopUp();
     assert_true(popUp.matches(':open'));
     popUp.hidePopUp();
     assert_false(popUp.matches(':open'));
-  }, 'Show event is cancelable');
+  }, 'Popupshow event is cancelable');
 };
 </script>

blink/web_tests/external/wpt/html/semantics/popups/popup-invoking-attribute.tentative.html

@@ -175,8 +175,8 @@
 const button = document.querySelector('button');
 let showCount = 0;
 let hideCount = 0;
-popUp.addEventListener('show',() => ++showCount);
-popUp.addEventListener('hide',() => ++hideCount);
+popUp.addEventListener('popupshow',() => ++showCount);
+popUp.addEventListener('popuphide',() => ++hideCount);
 
 async function assertState(expectOpen,expectShow,expectHide) {
   assert_equals(popUp.matches(':open'),expectOpen,'Popup open state is incorrect');

blink/web_tests/external/wpt/html/semantics/popups/popup-light-dismiss-on-scroll.tentative.html

@@ -44,7 +44,7 @@
   }
   async_test(t => {
     for(let popup of popups) {
-      popup.addEventListener('hide',e => {
+      popup.addEventListener('popuphide',e => {
         assert_unreached('Scrolling should not light-dismiss a popup');
       });
     }

blink/web_tests/external/wpt/html/semantics/popups/popup-light-dismiss.tentative.html

@@ -46,14 +46,14 @@
   const afterp1 = document.querySelector('#after_p1');
 
   let popup1HideCount = 0;
-  popup1.addEventListener('hide',(e) => {
+  popup1.addEventListener('popuphide',(e) => {
     ++popup1HideCount;
-    e.preventDefault(); // 'hide' should not be cancellable.
+    e.preventDefault(); // 'popuphide' should not be cancellable.
   });
   let popup2HideCount = 0;
-  popup2.addEventListener('hide',(e) => {
+  popup2.addEventListener('popuphide',(e) => {
     ++popup2HideCount;
-    e.preventDefault(); // 'hide' should not be cancellable.
+    e.preventDefault(); // 'popuphide' should not be cancellable.
   });
   promise_test(async () => {
     assert_false(popup1.matches(':open'));

blink/web_tests/external/wpt/html/semantics/popups/popup-show-event.tentative.html

@@ -14,7 +14,7 @@
   requestAnimationFrame(() => {++frameCount;});
   const popUp = document.querySelector('[popup]');
   const testText = 'Show Event Occurred';
-  popUp.addEventListener('show',() => {
+  popUp.addEventListener('popupshow',() => {
     popUp.textContent = testText;
   })
   popUp.offsetHeight;

blink/web_tests/fast/dom/Window/property-access-on-cached-window-after-frame-navigated-expected.txt

@@ -180,6 +180,8 @@ PASS oldChildWindow.onpointerover is newChildWindow.onpointerover
 PASS oldChildWindow.onpointerrawupdate is newChildWindow.onpointerrawupdate
 PASS oldChildWindow.onpointerup is newChildWindow.onpointerup
 PASS oldChildWindow.onpopstate is newChildWindow.onpopstate
+PASS oldChildWindow.onpopuphide is newChildWindow.onpopuphide
+PASS oldChildWindow.onpopupshow is newChildWindow.onpopupshow
 PASS oldChildWindow.onprogress is newChildWindow.onprogress
 PASS oldChildWindow.onratechange is newChildWindow.onratechange
 PASS oldChildWindow.onrejectionhandled is newChildWindow.onrejectionhandled

blink/web_tests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced-expected.txt

@@ -125,6 +125,8 @@ PASS childWindow.onpointerover is null
 PASS childWindow.onpointerrawupdate is null
 PASS childWindow.onpointerup is null
 PASS childWindow.onpopstate is null
+PASS childWindow.onpopuphide is null
+PASS childWindow.onpopupshow is null
 PASS childWindow.onprogress is null
 PASS childWindow.onratechange is null
 PASS childWindow.onrejectionhandled is null

blink/web_tests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-expected.txt

@@ -125,6 +125,8 @@ PASS childWindow.onpointerover is null
 PASS childWindow.onpointerrawupdate is null
 PASS childWindow.onpointerup is null
 PASS childWindow.onpopstate is null
+PASS childWindow.onpopuphide is null
+PASS childWindow.onpopupshow is null
 PASS childWindow.onprogress is null
 PASS childWindow.onratechange is null
 PASS childWindow.onrejectionhandled is null

blink/web_tests/webexposed/element-instance-property-listing-expected.txt

@@ -249,6 +249,8 @@ namespace http://www.w3.org/1999/xhtml
     property onpointerover
     property onpointerrawupdate
     property onpointerup
+    property onpopuphide
+    property onpopupshow
     property onprogress
     property onratechange
     property onreset
@@ -1480,6 +1482,8 @@ namespace http://www.w3.org/2000/svg
     property onpointerover
     property onpointerrawupdate
     property onpointerup
+    property onpopuphide
+    property onpopupshow
     property onprogress
     property onratechange
     property onreset