Skip to content

Commit

Permalink
Rename togglepopup->popuptoggletarget, etc.
Browse files Browse the repository at this point in the history
Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

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

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 <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1025053}
NOKEYCHECK=True
GitOrigin-RevId: ea7ffdfd7ac1a2a09c410996cdd8bc7bb9e326da
  • Loading branch information
Mason Freed authored and copybara-github committed Jul 17, 2022
1 parent 8575b36 commit 8621255
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 65 deletions.
24 changes: 12 additions & 12 deletions blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLFormControlElement>(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)) {
Expand All @@ -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);
Expand Down
30 changes: 15 additions & 15 deletions blink/renderer/core/html/forms/html_form_control_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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() ||
Expand All @@ -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.
}
Expand All @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions blink/renderer/core/html/forms/html_form_control_element.h
Original file line number Diff line number Diff line change
Expand Up @@ -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> element;
Expand All @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/html/html_attribute_names.json5
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@
"headers",
"height",
"hidden",
"hidepopup",
"high",
"hoverpopup",
"href",
Expand Down Expand Up @@ -288,6 +287,9 @@
"ping",
"policy",
"popup",
"popuphidetarget",
"popupshowtarget",
"popuptoggletarget",
"poster",
"preload",
"property",
Expand All @@ -313,7 +315,6 @@
"shadowroot",
"shadowrootdelegatesfocus",
"shape",
"showpopup",
"size",
"sizes",
"slot",
Expand All @@ -334,7 +335,6 @@
"title",
"topmargin",
"translate",
"togglepopup",
"truespeed",
"trusttoken",
"type",
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/modules/accessibility/ax_node_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLFormControlElement>(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;
}
Expand Down Expand Up @@ -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<HTMLFormControlElement>(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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
button.remove();
});
popUp.id = popUpId;
button.setAttribute('togglepopup', popUpId);
button.setAttribute('popuptoggletarget', popUpId);
return button;
}
function addPriorFocus(t) {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -184,7 +184,7 @@



<button togglepopup=p1>Toggle Popup 1</button>
<button popuptoggletarget=p1>Toggle Popup 1</button>
<div popup id=p1 style="border: 5px solid red;top: 100px;left: 100px;">This is pop-up #1</div>

<script>
Expand Down Expand Up @@ -221,7 +221,7 @@
await assertState(true,2,1);
popUp.hidePopUp();
await assertState(false,2,2);
}, "Clicking a togglepopup button opens a closed pop-up (also check event counts)");
}, "Clicking a popuptoggletarget button opens a closed pop-up (also check event counts)");

promise_test(async () => {
showCount = hideCount = 0;
Expand All @@ -230,4 +230,4 @@
await assertState(true,1,0);
await clickOn(button);
await assertState(false,1,1);
}, "Clicking a togglepopup button closes an open pop-up (also check event counts)");
}, "Clicking a popuptoggletarget button closes an open pop-up (also check event counts)");
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
<script src="/resources/testdriver-vendor.js"></script>
<script src="resources/popup-utils.js"></script>

<button id=b1t togglepopup='p1'>Popup 1</button>
<button id=b1s showpopup='p1'>Popup 1</button>
<button id=b1t popuptoggletarget='p1'>Popup 1</button>
<button id=b1s popupshowtarget='p1'>Popup 1</button>
<button id=p1anchor>Popup1 anchor (no action)</button>
<span id=outside>Outside all popups</span>
<div popup id=p1 anchor=p1anchor>
<span id=inside1>Inside popup 1</span>
<button id=b2 togglepopup='p2'>Popup 2</button>
<button id=b2 popuptoggletarget='p2'>Popup 2</button>
<span id=inside1after>Inside popup 1 after button</span>
</div>
<div popup id=p2 anchor=b2>
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -180,11 +180,11 @@
},'Dragging from an open popup outside an open popup should leave the popup open');
</script>

<button id=b3 togglepopup=p3>Popup 3 - button 3
<button id=b3 popuptoggletarget=p3>Popup 3 - button 3
<div popup id=p4>Inside popup 4</div>
</button>
<div popup id=p3>Inside popup 3</div>
<button id=b4 togglepopup=p3>Popup 3 - button 4
<button id=b4 popuptoggletarget=p3>Popup 3 - button 4
<div popup id=p5>Inside popup 5</div>
</button>
<style>
Expand Down Expand Up @@ -228,7 +228,7 @@
<div style="height:2000px;background:lightgreen"></div>
Bottom of popup6
</div>
<button togglepopup=p6>Popup 6</button>
<button popuptoggletarget=p6>Popup 6</button>
<style>
#p6 {
width: 300px;
Expand Down Expand Up @@ -308,7 +308,7 @@
<button>Button</button>
<span id=inside9after>Inside popup 9 after button</span>
</div>
<button id=b9after togglepopup='p9'>Popup 9</button>
<button id=b9after popuptoggletarget='p9'>Popup 9</button>
<script>
promise_test(async () => {
const popup9 = document.querySelector('#p9');
Expand Down Expand Up @@ -342,15 +342,15 @@
<!-- Convoluted ancestor relationship -->
<div popup id=convoluted_p1>Popup 1
<div id=convoluted_anchor>Anchor
<button togglepopup=convoluted_p2>Open Popup 2</button>
<button popuptoggletarget=convoluted_p2>Open Popup 2</button>
<div popup id=convoluted_p4><p>Popup 4</p></div>
</div>
</div>
<div popup id=convoluted_p2>Popup 2
<button togglepopup=convoluted_p3>Open Popup 3</button>
<button popuptoggletarget=convoluted_p3>Open Popup 3</button>
</div>
<div popup id=convoluted_p3 anchor=convoluted_anchor>Popup 3
<button togglepopup=convoluted_p4>Open Popup 4</button>
<button popuptoggletarget=convoluted_p4>Open Popup 4</button>
</div>
<button onclick="convoluted_p1.showPopUp()">Open convoluted popup</button>
<style>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,19 @@
</div>

<div class="example">
<p>togglepopup attribute relationship</p>
<p>popuptoggletarget attribute relationship</p>
<div popup class=ancestor><p>Ancestor popup</p>
<button togglepopup=trigger1 class=clickme>Button</button>
<button popuptoggletarget=trigger1 class=clickme>Button</button>
</div>
<div id=trigger1 popup class=child><p>Child popup</p></div>
</div>

<div class="example">
<p>nested togglepopup attribute relationship</p>
<p>nested popuptoggletarget attribute relationship</p>
<div popup class=ancestor><p>Ancestor popup</p>
<div>
<div>
<button togglepopup=trigger2 class=clickme>Button</button>
<button popuptoggletarget=trigger2 class=clickme>Button</button>
</div>
</div>
</div>
Expand Down

0 comments on commit 8621255

Please sign in to comment.