Skip to content

Commit

Permalink
Add anchor content attribute and hook up to popups [popup 6/N]
Browse files Browse the repository at this point in the history
This CL adds support for the 'anchor' content attribute generally on
Element, and hooks it up to ancestor popup processing for popups.

This CL also enables the code that closes all popups when a <dialog>
is shown.

The test changes here are mostly cleanup and adding more cases, plus
fixing a few bugs that crept in to the copy/paste from <popup> element
tests.

Bug: 1307772
Change-Id: I3032d08ec30866290f10d2845b77f0c6e9037a76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561704
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988036}
  • Loading branch information
Mason Freed authored and Chromium LUCI CQ committed Apr 1, 2022
1 parent f106c44 commit d997198
Show file tree
Hide file tree
Showing 17 changed files with 124 additions and 102 deletions.
35 changes: 20 additions & 15 deletions third_party/blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2436,6 +2436,8 @@ void Element::hidePopup() {

// static
const Element* Element::NearestOpenAncestralPopup(Node* start_node) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupElementEnabled() ||
RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
if (!start_node)
return nullptr;
// We need to walk up from the start node to see if there is a parent popup,
Expand All @@ -2445,22 +2447,13 @@ const Element* Element::NearestOpenAncestralPopup(Node* start_node) {
// popup, but we will stop on any of them. Therefore, just store the popup
// that is highest (last) on the popup stack for each anchor and/or invoker.

// TODO(masonf): getAnchor and getInvoker can be removed once the
// HTMLPopupElement is removed.
auto getAnchor = [](const Element* element) -> Element* {
if (auto* popup_element = DynamicTo<HTMLPopupElement>(element)) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupElementEnabled());
return popup_element->anchor();
} else {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
// TODO(masonf): Implement the anchor attribute for general elements.
return nullptr;
}
};
// TODO(masonf): getInvoker can be removed once the HTMLPopupElement is
// removed.
auto getInvoker = [](const Element* element) -> Element* {
if (auto* popup_element = DynamicTo<HTMLPopupElement>(element)) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupElementEnabled());
return popup_element->invoker();
// The <popup> element `popup` attribute has been deprecated and removed.
return nullptr;
} else {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
return element->GetPopupData()->invoker();
Expand All @@ -2471,7 +2464,7 @@ const Element* Element::NearestOpenAncestralPopup(Node* start_node) {
anchors_and_invokers;
Document& document = start_node->GetDocument();
for (auto popup : document.PopupElementStack()) {
if (const auto* anchor = getAnchor(popup))
if (const auto* anchor = popup->anchorElement())
anchors_and_invokers.Set(anchor, popup);
if (const auto* invoker = getInvoker(popup))
anchors_and_invokers.Set(invoker, popup);
Expand Down Expand Up @@ -2500,7 +2493,7 @@ const Element* Element::NearestOpenAncestralPopup(Node* start_node) {
if (IsA<HTMLPopupElement>(start_element) ||
start_element->HasValidPopupAttribute()) {
if (auto* anchor_ancestor =
NearestOpenAncestralPopup(getAnchor(start_element))) {
NearestOpenAncestralPopup(start_element->anchorElement())) {
return anchor_ancestor;
}
if (auto* invoker_ancestor =
Expand Down Expand Up @@ -2551,6 +2544,18 @@ void Element::InvokePopup(Element* invoker) {
showPopup();
}

Element* Element::anchorElement() const {
if (!RuntimeEnabledFeatures::HTMLPopupAttributeEnabled() &&
!RuntimeEnabledFeatures::HTMLPopupElementEnabled()) {
return nullptr;
}
const AtomicString& anchor_id = FastGetAttribute(html_names::kAnchorAttr);
if (anchor_id.IsNull())
return nullptr;
if (!IsInTreeScope())
return nullptr;
return GetTreeScope().getElementById(anchor_id); // may be null
}
bool Element::HasLegalLinkAttribute(const QualifiedName&) const {
return false;
}
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/dom/element.h
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,10 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
kAttachLayoutTree,
};

// Retrieves the element pointed to by this element's 'anchor' content
// attribute, if that element exists.
Element* anchorElement() const;

void UpdateFirstLetterPseudoElement(StyleUpdatePhase,
const StyleRecalcContext&);

Expand Down
6 changes: 4 additions & 2 deletions third_party/blink/renderer/core/html/html_dialog_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ void HTMLDialogElement::show() {
SetBooleanAttribute(html_names::kOpenAttr, true);

// Showing a <dialog> should hide all open popups.
if (RuntimeEnabledFeatures::HTMLPopupElementEnabled()) {
if (RuntimeEnabledFeatures::HTMLPopupElementEnabled() ||
RuntimeEnabledFeatures::HTMLPopupAttributeEnabled()) {
GetDocument().HideAllPopupsUntil(nullptr);
}

Expand Down Expand Up @@ -213,7 +214,8 @@ void HTMLDialogElement::showModal(ExceptionState& exception_state) {
}

// Showing a <dialog> should hide all open popups.
if (RuntimeEnabledFeatures::HTMLPopupElementEnabled()) {
if (RuntimeEnabledFeatures::HTMLPopupElementEnabled() ||
RuntimeEnabledFeatures::HTMLPopupAttributeEnabled()) {
document.HideAllPopupsUntil(nullptr);
}

Expand Down
24 changes: 0 additions & 24 deletions third_party/blink/renderer/core/html/html_popup_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ 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) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupElementEnabled());
Expand All @@ -48,7 +47,6 @@ void HTMLPopupElement::hide() {
if (!open_)
return;
open_ = false;
invoker_ = nullptr;
needs_repositioning_for_select_menu_ = false;
DCHECK(isConnected());
GetDocument().HideAllPopupsUntil(this);
Expand All @@ -64,12 +62,6 @@ void HTMLPopupElement::ScheduleHideEvent() {
GetDocument().EnqueueAnimationFrameEvent(event);
}

void HTMLPopupElement::Invoke(Element* invoker) {
DCHECK(invoker);
invoker_ = invoker;
show();
}

void HTMLPopupElement::show() {
if (open_ || !isConnected())
return;
Expand Down Expand Up @@ -221,21 +213,6 @@ void HTMLPopupElement::PopPopupElement(HTMLPopupElement* popup) {
GetDocument().RemoveFromTopLayer(popup);
}

Element* HTMLPopupElement::anchor() const {
const AtomicString& anchor_id = FastGetAttribute(html_names::kAnchorAttr);
if (anchor_id.IsNull())
return nullptr;
if (!IsInTreeScope())
return nullptr;
if (Element* anchor = GetTreeScope().getElementById(anchor_id))
return anchor;
return nullptr;
}

Element* HTMLPopupElement::invoker() const {
return invoker_.Get();
}

// TODO(crbug.com/1197720): The popup position should be provided by the new
// anchored positioning scheme.
void HTMLPopupElement::SetNeedsRepositioningForSelectMenu(bool flag) {
Expand Down Expand Up @@ -329,7 +306,6 @@ void HTMLPopupElement::AdjustPopupPositionForSelectMenu(ComputedStyle& style) {
}

void HTMLPopupElement::Trace(Visitor* visitor) const {
visitor->Trace(invoker_);
visitor->Trace(owner_select_menu_element_);
HTMLElement::Trace(visitor);
}
Expand Down
8 changes: 0 additions & 8 deletions third_party/blink/renderer/core/html/html_popup_element.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ class HTMLPopupElement final : public HTMLElement {
void hide();
void show();

Element* anchor() const;
Element* invoker() const;

// This is used by invoking elements (which have a "popup" attribute)
// to invoke the popup.
void Invoke(Element* invoker);

// TODO(crbug.com/1197720): The popup position should be provided by the new
// anchored positioning scheme.
void SetNeedsRepositioningForSelectMenu(bool flag);
Expand Down Expand Up @@ -70,7 +63,6 @@ class HTMLPopupElement final : public HTMLElement {

bool open_;
bool had_initiallyopen_when_parsed_;
WeakMember<Element> invoker_;

bool needs_repositioning_for_select_menu_;
WeakMember<HTMLSelectMenuElement> owner_select_menu_element_;
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/renderer/core/html/html_popup_element.idl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,4 @@ interface HTMLPopupElement : HTMLElement {
[CEReactions, Reflect] attribute boolean initiallyOpen;
[CEReactions, Measure] void show();
[CEReactions, Measure] void hide();

readonly attribute Element anchor;
};

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>

<button id=b1 onclick='p1.showPopup()'>Popup 1</button>
<button id=b1 triggerpopup='p1'>Popup 1</button>
<span id=outside>Outside all popups</span>
<div popup=popup id=p1 anchor=b1>
<span id=inside1>Inside popup 1</span>
<button id=b2 onclick='p2.showPopup()'>Popup 2</button>
<button id=b2 triggerpopup='p2'>Popup 2</button>
</div>
<div popup=popup id=p2 anchor=b2>
<span id=inside2>Inside popup 2</span>
</div>

<button id=b3 popup=p3>Popup 3 - button 3
<button id=b3 triggerpopup=p3>Popup 3 - button 3
<div popup=popup id=p4>Inside popup 4</div>
</button>
<div popup=popup id=p3>Inside popup 3</div>
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,67 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<!-- Enumerate all the ways of creating an ancestor popup relationship -->

<div class="example">
<p>Direct DOM children</p>
<div popup=popup class=ancestor><p>Ancestor popup</p>
<div popup=popup class=child><p>Child popup</p></div>
</div>
</div>

<div class="example">
<p>Grandchildren</p>
<div popup=popup class=ancestor><p>Ancestor popup</p>
<div>
<div>
<div popup=popup class=child><p>Child popup</p></div>
</div>
</div>
</div>
</div>

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

<div class="example">
<p>nested triggerpopup attribute relationship</p>
<div popup=popup class=ancestor><p>Ancestor popup</p>
<div>
<div>
<button triggerpopup=trigger2 class=clickme>Button</button>
</div>
</div>
</div>
<div id=trigger2 popup=popup class=child><p>Child popup</p></div>
</div>

<div class="example">
<p>anchor attribute relationship</p>
<div id=anchor1 popup=popup class=ancestor><p>Ancestor popup</p></div>
<div anchor=anchor1 popup=popup class=child><p>Child popup</p></div>
</div>

<div class="example">
<p>indirect anchor attribute relationship</p>
<div popup=popup class=ancestor>
<p>Ancestor popup</p>
<div>
<div>
<span id=anchor2>Anchor</span>
</div>
</div>
</div>
<div anchor=anchor2 popup=popup class=child><p>Child popup</p></div>
</div>

<!-- Other examples -->

<div popup=popup id=p1 anchor=b1><p>This is popup #1</p>
<button id=b2 onclick='p2.showPopup()'>Popup 2</button>
<button id=b4 onclick='p4.showPopup()'>Popup 4</button>
Expand All @@ -20,6 +81,29 @@
<button id=b5 onclick='d1.showPopup()'>Dialog</button>

<script>
// Test basic ancestor relationships
for(let example of document.querySelectorAll('.example')) {
const descr = example.querySelector('p').textContent;
const ancestor = example.querySelector('[popup].ancestor');
const child = example.querySelector('[popup].child');
const clickToActivate = example.querySelector('.clickme');
test(function() {
assert_true(!!descr && !!ancestor && !!child);
assert_false(ancestor.matches(':popup-open'));
assert_false(child.matches(':popup-open'));
ancestor.showPopup();
if (clickToActivate)
clickToActivate.click();
else
child.showPopup();
assert_true(child.matches(':popup-open'));
assert_true(ancestor.matches(':popup-open'));
ancestor.hidePopup();
assert_false(ancestor.matches(':popup-open'));
assert_false(child.matches(':popup-open'));
},descr);
}

const popups = [p1, p2, p3, p4];

function assertState(...states) {
Expand All @@ -46,7 +130,7 @@
// Hiding P1 should hide all.
p1.hidePopup();
assertState(false,false,false,false);
}, "popup stacking behavior")
}, "more complex nesting, all using anchor ancestry")

test(function() {
function openManyPopups() {
Expand All @@ -56,7 +140,7 @@
assertState(true,true,true,false);
}
openManyPopups();
d1.showPopup(); // Dialog.showPopup() should hide all popups.
d1.show(); // Dialog.showPopup() should hide all popups.
assertState(false,false,false,false);
d1.close();
openManyPopups();
Expand All @@ -66,14 +150,17 @@
}, "popups should be closed by dialogs")

test(function() {
d1.showPopup();
assert_true(d1.matches(':popup-open'));
// Note: d1 is a <dialog> element, not a popup.
assert_false(d1.open);
d1.show();
assert_true(d1.open);
p1.showPopup();
assertState(true,false,false,false);
assert_true(d1.matches(':popup-open'));
assert_true(d1.open);
p1.hidePopup();
assert_true(d1.matches(':popup-open'));
assert_true(d1.open);
d1.close();
assert_false(d1.open);
}, "dialogs should not be closed by popups")
</script>

Expand Down

0 comments on commit d997198

Please sign in to comment.