From 86212556c57dd0ea286afd5686b45bc4154745ce Mon Sep 17 00:00:00 2001 From: Mason Freed Date: Sat, 16 Jul 2022 23:53:11 +0000 Subject: [PATCH] Rename togglepopup->popuptoggletarget, etc. Per the resolution [1], we are renaming: - togglepopup -> popuptoggletarget - showpopup -> popupshowtarget - hidepopup -> popuphidetarget A subsequent CL will add IDL reflections of these attributes. [1] https://github.com/openui/open-ui/issues/382#issuecomment-1184773425 Bug: 1307772 Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0 Cq-Do-Not-Cancel-Tryjobs: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764624 Auto-Submit: Mason Freed Reviewed-by: Joey Arhar Reviewed-by: Nektarios Paisios Commit-Queue: Nektarios Paisios Cr-Commit-Position: refs/heads/main@{#1025053} NOKEYCHECK=True GitOrigin-RevId: ea7ffdfd7ac1a2a09c410996cdd8bc7bb9e326da --- blink/renderer/core/dom/element.cc | 24 +++++++-------- .../html/forms/html_form_control_element.cc | 30 +++++++++---------- .../html/forms/html_form_control_element.h | 10 +++---- .../core/html/html_attribute_names.json5 | 6 ++-- .../modules/accessibility/ax_node_object.cc | 4 +-- .../popups/popup-focus.tentative.html | 10 +++---- .../popup-invoking-attribute.tentative.html | 12 ++++---- .../popups/popup-light-dismiss.tentative.html | 26 ++++++++-------- .../popups/popup-stacking.tentative.html | 8 ++--- 9 files changed, 65 insertions(+), 65 deletions(-) diff --git a/blink/renderer/core/dom/element.cc b/blink/renderer/core/dom/element.cc index 82d73e4c682..a2273d2fa78 100644 --- a/blink/renderer/core/dom/element.cc +++ b/blink/renderer/core/dom/element.cc @@ -2906,7 +2906,7 @@ const Element* NearestOpenAncestralPopupRecursive( // to catch invoking elements that should not light dismiss a pop-up, even // if they weren't used to show it. if (auto* form_control = DynamicTo(element)) { - recurse_and_update(form_control->togglePopupElement().element); + recurse_and_update(form_control->popupTargetElement().element); } // Include the anchor elements for all showing pop-ups. if (anchors_to_popups.Contains(element)) { @@ -2926,17 +2926,17 @@ const Element* NearestOpenAncestralPopupRecursive( // DOM flat tree parents, or through either anchor or invoker relationships. // Anchor relationships are formed by the anchor attribute on a pop-up, pointing // to another node in the tree. Invoker relationships are formed by invoking -// elements, which are HTMLFormControlElements having togglepopup, showpopup, or -// hidepopup attributes pointing to a pop-up element. There can be multiple -// pop-ups that point to a single anchor element, and there can be multiple -// invoking elements for a single pop-up. Additionally, an anchor for one pop-up -// can be an invoker for a different pop-up. For these reasons, this function -// needs to do a recursive tree walk up from the provided node, plus all -// associated anchors and invokers, returning the highest (on the stack) pop-up -// that is found. If the inclusive parameter is true, the highest pop-up found -// during the tree-walk is included in the search. If it is false, the |node| -// parameter must be a pop-up, and the highest pop-up *below* that starting pop- -// up will be returned. +// elements, which are HTMLFormControlElements having popuptoggletarget, +// popupshowtarget, or popuphidetarget attributes pointing to a pop-up element. +// There can be multiple pop-ups that point to a single anchor element, and +// there can be multiple invoking elements for a single pop-up. Additionally, an +// anchor for one pop-up can be an invoker for a different pop-up. For these +// reasons, this function needs to do a recursive tree walk up from the provided +// node, plus all associated anchors and invokers, returning the highest (on the +// stack) pop-up that is found. If the inclusive parameter is true, the highest +// pop-up found during the tree-walk is included in the search. If it is false, +// the |node| parameter must be a pop-up, and the highest pop-up *below* that +// starting pop- up will be returned. const Element* Element::NearestOpenAncestralPopup(const Node* node, bool inclusive) { DCHECK(node); diff --git a/blink/renderer/core/html/forms/html_form_control_element.cc b/blink/renderer/core/html/forms/html_form_control_element.cc index 55b099dbddf..cac04e3883e 100644 --- a/blink/renderer/core/html/forms/html_form_control_element.cc +++ b/blink/renderer/core/html/forms/html_form_control_element.cc @@ -328,9 +328,9 @@ bool HTMLFormControlElement::IsSuccessfulSubmitButton() const { // used. // 4. If both 'showpopup' and 'hidepopup' are present, the behavior is to // toggle. -HTMLFormControlElement::TogglePopupElement -HTMLFormControlElement::togglePopupElement() const { - const TogglePopupElement no_element{.element = nullptr, +HTMLFormControlElement::PopupTargetElement +HTMLFormControlElement::popupTargetElement() const { + const PopupTargetElement no_element{.element = nullptr, .action = PopupTriggerAction::kNone, .attribute_name = g_null_name}; if (!RuntimeEnabledFeatures::HTMLPopupAttributeEnabled() || @@ -340,21 +340,21 @@ HTMLFormControlElement::togglePopupElement() const { } AtomicString idref; - QualifiedName attribute_name = html_names::kTogglepopupAttr; + QualifiedName attribute_name = html_names::kPopuptoggletargetAttr; PopupTriggerAction action = PopupTriggerAction::kToggle; - if (FastHasAttribute(html_names::kTogglepopupAttr)) { - idref = FastGetAttribute(html_names::kTogglepopupAttr); - } else if (FastHasAttribute(html_names::kShowpopupAttr)) { - idref = FastGetAttribute(html_names::kShowpopupAttr); + if (FastHasAttribute(html_names::kPopuptoggletargetAttr)) { + idref = FastGetAttribute(html_names::kPopuptoggletargetAttr); + } else if (FastHasAttribute(html_names::kPopupshowtargetAttr)) { + idref = FastGetAttribute(html_names::kPopupshowtargetAttr); action = PopupTriggerAction::kShow; - attribute_name = html_names::kShowpopupAttr; + attribute_name = html_names::kPopupshowtargetAttr; } - if (FastHasAttribute(html_names::kHidepopupAttr)) { + if (FastHasAttribute(html_names::kPopuphidetargetAttr)) { if (idref.IsNull()) { - idref = FastGetAttribute(html_names::kHidepopupAttr); + idref = FastGetAttribute(html_names::kPopuphidetargetAttr); action = PopupTriggerAction::kHide; - attribute_name = html_names::kHidepopupAttr; - } else if (FastGetAttribute(html_names::kHidepopupAttr) == idref) { + attribute_name = html_names::kPopuphidetargetAttr; + } else if (FastGetAttribute(html_names::kPopuphidetargetAttr) == idref) { action = PopupTriggerAction::kToggle; // Leave attribute_name as-is in this case. } @@ -364,14 +364,14 @@ HTMLFormControlElement::togglePopupElement() const { Element* popup_element = GetTreeScope().getElementById(idref); if (!popup_element || !popup_element->HasValidPopupAttribute()) return no_element; - return TogglePopupElement{.element = popup_element, + return PopupTargetElement{.element = popup_element, .action = action, .attribute_name = attribute_name}; } void HTMLFormControlElement::DefaultEventHandler(Event& event) { if (!IsDisabledFormControl()) { - auto popup = togglePopupElement(); + auto popup = popupTargetElement(); if (popup.element) { auto trigger_support = SupportsPopupTriggering(); DCHECK_NE(popup.action, PopupTriggerAction::kNone); diff --git a/blink/renderer/core/html/forms/html_form_control_element.h b/blink/renderer/core/html/forms/html_form_control_element.h index efd53045256..e5a0088549b 100644 --- a/blink/renderer/core/html/forms/html_form_control_element.h +++ b/blink/renderer/core/html/forms/html_form_control_element.h @@ -88,7 +88,7 @@ class CORE_EXPORT HTMLFormControlElement : public HTMLElement, virtual bool IsActivatedSubmit() const { return false; } virtual void SetActivatedSubmit(bool) {} - struct TogglePopupElement final { + struct PopupTargetElement final { public: DISALLOW_NEW(); WeakMember element; @@ -103,10 +103,10 @@ class CORE_EXPORT HTMLFormControlElement : public HTMLElement, kDownArrow, }; - // Retrieves the element pointed to by 'togglepopup', 'showpopup', and/or - // 'hidepopup' content attributes, if any, and only if this form control - // element supports popup triggering. - TogglePopupElement togglePopupElement() const; + // Retrieves the element pointed to by 'popuptoggletarget', 'popupshowtarget', + // and/or 'popuphidetarget' content attributes, if any, and only if this form + // control element supports popup triggering. + PopupTargetElement popupTargetElement() const; virtual PopupTriggerSupport SupportsPopupTriggering() const { return PopupTriggerSupport::kNone; } diff --git a/blink/renderer/core/html/html_attribute_names.json5 b/blink/renderer/core/html/html_attribute_names.json5 index 057207abaab..7f73efc1744 100644 --- a/blink/renderer/core/html/html_attribute_names.json5 +++ b/blink/renderer/core/html/html_attribute_names.json5 @@ -100,7 +100,6 @@ "headers", "height", "hidden", - "hidepopup", "high", "hoverpopup", "href", @@ -288,6 +287,9 @@ "ping", "policy", "popup", + "popuphidetarget", + "popupshowtarget", + "popuptoggletarget", "poster", "preload", "property", @@ -313,7 +315,6 @@ "shadowroot", "shadowrootdelegatesfocus", "shape", - "showpopup", "size", "sizes", "slot", @@ -334,7 +335,6 @@ "title", "topmargin", "translate", - "togglepopup", "truespeed", "trusttoken", "type", diff --git a/blink/renderer/modules/accessibility/ax_node_object.cc b/blink/renderer/modules/accessibility/ax_node_object.cc index 0dddd5123a4..9aeb2475b8b 100644 --- a/blink/renderer/modules/accessibility/ax_node_object.cc +++ b/blink/renderer/modules/accessibility/ax_node_object.cc @@ -1850,7 +1850,7 @@ AccessibilityExpanded AXNodeObject::IsExpanded() const { // kPopup, then set aria-expanded=false when the popup is hidden, and // aria-expanded=true when it is showing. if (auto* form_control = DynamicTo(element)) { - if (auto popup = form_control->togglePopupElement().element; + if (auto popup = form_control->popupTargetElement().element; popup && popup->PopupType() == PopupValueType::kAuto) { return popup->popupOpen() ? kExpandedExpanded : kExpandedCollapsed; } @@ -5629,7 +5629,7 @@ String AXNodeObject::Description( // For form controls that act as triggering elements for popups of type kHint, // then set aria-describedby to the hint popup. if (auto* form_control = DynamicTo(element)) { - auto popup = form_control->togglePopupElement(); + auto popup = form_control->popupTargetElement(); if (popup.element && popup.element->PopupType() == PopupValueType::kHint) { description_from = ax::mojom::blink::DescriptionFrom::kPopupElement; if (description_sources) { diff --git a/blink/web_tests/external/wpt/html/semantics/popups/popup-focus.tentative.html b/blink/web_tests/external/wpt/html/semantics/popups/popup-focus.tentative.html index 0ab90bbf554..76cc33261ea 100644 --- a/blink/web_tests/external/wpt/html/semantics/popups/popup-focus.tentative.html +++ b/blink/web_tests/external/wpt/html/semantics/popups/popup-focus.tentative.html @@ -123,7 +123,7 @@ button.remove(); }); popUp.id = popUpId; - button.setAttribute('togglepopup', popUpId); + button.setAttribute('popuptoggletarget', popUpId); return button; } function addPriorFocus(t) { @@ -218,7 +218,7 @@ const priorFocus = addPriorFocus(t); assert_false(popUp.matches(':top-layer'), 'pop-up should start out hidden'); let button = addInvoker(t, popUp); - assert_equals(button.getAttribute('togglepopup'), popUp.id, 'This test assumes the button uses `togglepopup`.'); + assert_equals(button.getAttribute('popuptoggletarget'), popUp.id, 'This test assumes the button uses `popuptoggletarget`.'); assert_not_equals(button, priorFocus, 'Stranger things have happened'); assert_false(popUp.contains(button), 'Start with a non-contained button'); priorFocus.focus(); @@ -231,8 +231,8 @@ await finishAnimationsAndVerifyHide(popUp); // Same thing, but the button is contained within the pop-up - button.removeAttribute('togglepopup'); - button.setAttribute('hidepopup', popUp.id); + button.removeAttribute('popuptoggletarget'); + button.setAttribute('popuphidetarget', popUp.id); popUp.appendChild(button); priorFocus.focus(); popUp.showPopUp(); @@ -245,7 +245,7 @@ assert_equals(document.activeElement, priorFocus, 'Contained button should return focus to the previously focused element'); await finishAnimationsAndVerifyHide(popUp); - // Same thing, but the button is unrelated (no togglepopup) + // Same thing, but the button is unrelated (no popuptoggletarget) button = document.createElement('button'); document.body.appendChild(button); priorFocus.focus(); diff --git a/blink/web_tests/external/wpt/html/semantics/popups/popup-invoking-attribute.tentative.html b/blink/web_tests/external/wpt/html/semantics/popups/popup-invoking-attribute.tentative.html index 6b356c1563b..dbc3a1f4355 100644 --- a/blink/web_tests/external/wpt/html/semantics/popups/popup-invoking-attribute.tentative.html +++ b/blink/web_tests/external/wpt/html/semantics/popups/popup-invoking-attribute.tentative.html @@ -123,9 +123,9 @@ assert_equals(popUp2.popUp,type); assert_not_equals(popUp1.id,popUp2.id); const invoker = testcase.makeElement(test); - if (t) invoker.setAttribute('togglepopup',t===1 ? popUp1.id : popUp2.id); - if (s) invoker.setAttribute('showpopup',s===1 ? popUp1.id : popUp2.id); - if (h) invoker.setAttribute('hidepopup',h===1 ? popUp1.id : popUp2.id); + if (t) invoker.setAttribute('popuptoggletarget',t===1 ? popUp1.id : popUp2.id); + if (s) invoker.setAttribute('popupshowtarget',s===1 ? popUp1.id : popUp2.id); + if (h) invoker.setAttribute('popuphidetarget',h===1 ? popUp1.id : popUp2.id); assert_true(!document.getElementById(popUp1.id)); assert_true(!document.getElementById(popUp2.id)); document.body.appendChild(popUp1); @@ -184,7 +184,7 @@ - +
This is pop-up #1
- - + + Outside all popups
Inside popup 1 - + Inside popup 1 after button
@@ -145,9 +145,9 @@ await waitForRender(); p1HideCount = popup1HideCount; await clickOn(button1toggle); - assert_false(popup1.matches(':top-layer'),'popup1 should be hidden by togglepopup'); - assert_equals(popup1HideCount,p1HideCount+1,'popup1 should get hidden only once by togglepopup'); - },'Clicking on togglepopup element, even if it wasn\'t used for activation, should hide it exactly once'); + assert_false(popup1.matches(':top-layer'),'popup1 should be hidden by popuptoggletarget'); + assert_equals(popup1HideCount,p1HideCount+1,'popup1 should get hidden only once by popuptoggletarget'); + },'Clicking on popuptoggletarget element, even if it wasn\'t used for activation, should hide it exactly once'); promise_test(async () => { popup1.showPopUp(); @@ -180,11 +180,11 @@ },'Dragging from an open popup outside an open popup should leave the popup open'); -
Inside popup 3
-