Allow the <details> element to be opened and closed (#35261)

* Implement the <summary> element

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Implement UA shadow root for <details>

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Invalidate style when display is opened or closed

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Fix /_mozilla/mozilla/duplicated_scroll_ids.html

This test previously assumed that <details> elements would
not be rendered.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Implement implicit summary elements

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Update WPT expectations

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Remove test for duplicated scroll IDs

See https://github.com/servo/servo/pull/35261#discussion_r1969328725 for
reasoning.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Use Iterator::find to find implicit summary element

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This commit is contained in:
Simon Wülker 2025-02-25 12:56:36 +01:00 committed by GitHub
parent cceff77928
commit 754b117011
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 251 additions and 94 deletions

View file

@ -62,9 +62,6 @@ use xml5ever::serialize::TraversalScope::{
ChildrenOnly as XmlChildrenOnly, IncludeNode as XmlIncludeNode,
};
use super::customelementregistry::is_valid_custom_element_name;
use super::htmltablecolelement::{HTMLTableColElement, HTMLTableColElementLayoutHelpers};
use super::intersectionobserver::{IntersectionObserver, IntersectionObserverRegistration};
use crate::dom::activation::Activatable;
use crate::dom::attr::{Attr, AttrHelpersForLayout};
use crate::dom::bindings::cell::{ref_filter_map, DomRefCell, Ref, RefMut};
@ -94,7 +91,8 @@ use crate::dom::bindings::xmlname::{
use crate::dom::characterdata::CharacterData;
use crate::dom::create::create_element;
use crate::dom::customelementregistry::{
CallbackReaction, CustomElementDefinition, CustomElementReaction, CustomElementState,
is_valid_custom_element_name, CallbackReaction, CustomElementDefinition, CustomElementReaction,
CustomElementState,
};
use crate::dom::document::{
determine_policy_for_token, Document, LayoutDocumentHelpers, ReflowTriggerCondition,
@ -128,6 +126,7 @@ use crate::dom::htmlselectelement::HTMLSelectElement;
use crate::dom::htmlslotelement::{HTMLSlotElement, Slottable};
use crate::dom::htmlstyleelement::HTMLStyleElement;
use crate::dom::htmltablecellelement::{HTMLTableCellElement, HTMLTableCellElementLayoutHelpers};
use crate::dom::htmltablecolelement::{HTMLTableColElement, HTMLTableColElementLayoutHelpers};
use crate::dom::htmltableelement::{HTMLTableElement, HTMLTableElementLayoutHelpers};
use crate::dom::htmltablerowelement::{HTMLTableRowElement, HTMLTableRowElementLayoutHelpers};
use crate::dom::htmltablesectionelement::{
@ -136,6 +135,7 @@ use crate::dom::htmltablesectionelement::{
use crate::dom::htmltemplateelement::HTMLTemplateElement;
use crate::dom::htmltextareaelement::{HTMLTextAreaElement, LayoutHTMLTextAreaElementHelpers};
use crate::dom::htmlvideoelement::{HTMLVideoElement, LayoutHTMLVideoElementHelpers};
use crate::dom::intersectionobserver::{IntersectionObserver, IntersectionObserverRegistration};
use crate::dom::mutationobserver::{Mutation, MutationObserver};
use crate::dom::namednodemap::NamedNodeMap;
use crate::dom::node::{

View file

@ -2,29 +2,58 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use std::cell::Cell;
use std::cell::{Cell, Ref};
use dom_struct::dom_struct;
use html5ever::{local_name, LocalName, Prefix};
use js::rust::HandleObject;
use crate::dom::attr::Attr;
use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::codegen::Bindings::HTMLDetailsElementBinding::HTMLDetailsElementMethods;
use crate::dom::bindings::codegen::Bindings::HTMLSlotElementBinding::HTMLSlotElement_Binding::HTMLSlotElementMethods;
use crate::dom::bindings::codegen::Bindings::NodeBinding::Node_Binding::NodeMethods;
use crate::dom::bindings::codegen::Bindings::ShadowRootBinding::{
ShadowRootMode, SlotAssignmentMode,
};
use crate::dom::bindings::codegen::UnionTypes::ElementOrText;
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::root::DomRoot;
use crate::dom::bindings::root::{Dom, DomRoot};
use crate::dom::document::Document;
use crate::dom::element::AttributeMutation;
use crate::dom::element::{AttributeMutation, Element};
use crate::dom::eventtarget::EventTarget;
use crate::dom::htmlelement::HTMLElement;
use crate::dom::node::{Node, NodeDamage, NodeTraits};
use crate::dom::htmlslotelement::HTMLSlotElement;
use crate::dom::node::{BindContext, ChildrenMutation, Node, NodeDamage, NodeTraits};
use crate::dom::shadowroot::IsUserAgentWidget;
use crate::dom::text::Text;
use crate::dom::virtualmethods::VirtualMethods;
use crate::script_runtime::CanGc;
/// The summary that should be presented if no `<summary>` element is present
const DEFAULT_SUMMARY: &str = "Details";
/// Holds handles to all slots in the UA shadow tree
///
/// The composition of the tree is described in
/// <https://html.spec.whatwg.org/multipage/#the-details-and-summary-elements>
#[derive(Clone, JSTraceable, MallocSizeOf)]
#[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)]
struct ShadowTree {
summary: Dom<HTMLSlotElement>,
descendants: Dom<HTMLSlotElement>,
/// The summary that is displayed if no other summary exists
implicit_summary: Dom<HTMLElement>,
}
#[dom_struct]
pub(crate) struct HTMLDetailsElement {
htmlelement: HTMLElement,
toggle_counter: Cell<u32>,
/// Represents the UA widget for the details element
shadow_tree: DomRefCell<Option<ShadowTree>>,
}
impl HTMLDetailsElement {
@ -36,6 +65,7 @@ impl HTMLDetailsElement {
HTMLDetailsElement {
htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
toggle_counter: Cell::new(0),
shadow_tree: Default::default(),
}
}
@ -60,6 +90,131 @@ impl HTMLDetailsElement {
pub(crate) fn toggle(&self) {
self.SetOpen(!self.Open());
}
fn shadow_tree(&self, can_gc: CanGc) -> Ref<'_, ShadowTree> {
if !self.upcast::<Element>().is_shadow_host() {
self.create_shadow_tree(can_gc);
}
Ref::filter_map(self.shadow_tree.borrow(), Option::as_ref)
.ok()
.expect("UA shadow tree was not created")
}
fn create_shadow_tree(&self, can_gc: CanGc) {
let document = self.owner_document();
let root = self
.upcast::<Element>()
.attach_shadow(
IsUserAgentWidget::Yes,
ShadowRootMode::Closed,
false,
SlotAssignmentMode::Manual,
can_gc,
)
.expect("Attaching UA shadow root failed");
let summary = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc);
root.upcast::<Node>()
.AppendChild(summary.upcast::<Node>())
.unwrap();
let fallback_summary =
HTMLElement::new(local_name!("summary"), None, &document, None, can_gc);
fallback_summary
.upcast::<Node>()
.SetTextContent(Some(DEFAULT_SUMMARY.into()), can_gc);
summary
.upcast::<Node>()
.AppendChild(fallback_summary.upcast::<Node>())
.unwrap();
let descendants = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc);
root.upcast::<Node>()
.AppendChild(descendants.upcast::<Node>())
.unwrap();
let _ = self.shadow_tree.borrow_mut().insert(ShadowTree {
summary: summary.as_traced(),
descendants: descendants.as_traced(),
implicit_summary: fallback_summary.as_traced(),
});
self.upcast::<Node>()
.dirty(crate::dom::node::NodeDamage::OtherNodeDamage);
}
pub(crate) fn find_corresponding_summary_element(&self) -> Option<DomRoot<HTMLElement>> {
self.upcast::<Node>()
.children()
.filter_map(DomRoot::downcast::<HTMLElement>)
.find(|html_element| {
html_element.upcast::<Element>().local_name() == &local_name!("summary")
})
}
fn update_shadow_tree_contents(&self, can_gc: CanGc) {
let shadow_tree = self.shadow_tree(can_gc);
if let Some(summary) = self.find_corresponding_summary_element() {
shadow_tree
.summary
.Assign(vec![ElementOrText::Element(DomRoot::upcast(summary))]);
}
let mut slottable_children = vec![];
for child in self.upcast::<Node>().children() {
if let Some(element) = child.downcast::<Element>() {
if element.local_name() == &local_name!("summary") {
continue;
}
slottable_children.push(ElementOrText::Element(DomRoot::from_ref(element)));
}
if let Some(text) = child.downcast::<Text>() {
slottable_children.push(ElementOrText::Text(DomRoot::from_ref(text)));
}
}
shadow_tree.descendants.Assign(slottable_children);
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
}
fn update_shadow_tree_styles(&self, can_gc: CanGc) {
let shadow_tree = self.shadow_tree(can_gc);
let value = if self.Open() {
"display: block;"
} else {
// TODO: This should be "display: block; content-visibility: hidden;",
// but servo does not support content-visibility yet
"display: none;"
};
shadow_tree
.descendants
.upcast::<Element>()
.set_string_attribute(&local_name!("style"), value.into(), can_gc);
// Manually update the list item style of the implicit summary element.
// Unlike the other summaries, this summary is in the shadow tree and
// can't be styled with UA sheets
let implicit_summary_list_item_style = if self.Open() {
"disclosure-open"
} else {
"disclosure-closed"
};
let implicit_summary_style = format!(
"display: list-item;
counter-increment: list-item 0;
list-style: {implicit_summary_list_item_style} inside;"
);
shadow_tree
.implicit_summary
.upcast::<Element>()
.set_string_attribute(&local_name!("style"), implicit_summary_style.into(), can_gc);
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
}
}
impl HTMLDetailsElementMethods<crate::DomTypeHolder> for HTMLDetailsElement {
@ -79,6 +234,8 @@ impl VirtualMethods for HTMLDetailsElement {
self.super_type().unwrap().attribute_mutated(attr, mutation);
if attr.local_name() == &local_name!("open") {
self.update_shadow_tree_styles(CanGc::note());
let counter = self.toggle_counter.get() + 1;
self.toggle_counter.set(counter);
@ -92,7 +249,20 @@ impl VirtualMethods for HTMLDetailsElement {
this.upcast::<EventTarget>().fire_event(atom!("toggle"), CanGc::note());
}
}));
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage)
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
}
}
fn children_changed(&self, mutation: &ChildrenMutation) {
self.super_type().unwrap().children_changed(mutation);
self.update_shadow_tree_contents(CanGc::note());
}
fn bind_to_tree(&self, context: &BindContext) {
self.super_type().unwrap().bind_to_tree(context);
self.update_shadow_tree_contents(CanGc::note());
self.update_shadow_tree_styles(CanGc::note());
}
}

View file

@ -23,6 +23,7 @@ use crate::dom::bindings::codegen::Bindings::EventHandlerBinding::{
use crate::dom::bindings::codegen::Bindings::HTMLElementBinding::HTMLElementMethods;
use crate::dom::bindings::codegen::Bindings::HTMLLabelElementBinding::HTMLLabelElementMethods;
use crate::dom::bindings::codegen::Bindings::NodeBinding::Node_Binding::NodeMethods;
use crate::dom::bindings::codegen::Bindings::ShadowRootBinding::ShadowRoot_Binding::ShadowRootMethods;
use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
use crate::dom::bindings::error::{Error, ErrorResult, Fallible};
use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId};
@ -48,6 +49,7 @@ use crate::dom::htmlinputelement::{HTMLInputElement, InputType};
use crate::dom::htmllabelelement::HTMLLabelElement;
use crate::dom::htmltextareaelement::HTMLTextAreaElement;
use crate::dom::node::{BindContext, Node, NodeTraits, ShadowIncluding, UnbindContext};
use crate::dom::shadowroot::ShadowRoot;
use crate::dom::text::Text;
use crate::dom::virtualmethods::VirtualMethods;
use crate::script_runtime::CanGc;
@ -914,45 +916,64 @@ impl HTMLElement {
// https://html.spec.whatwg.org/multipage/#the-summary-element:activation-behaviour
pub(crate) fn summary_activation_behavior(&self) {
// Step 1
if !self.is_summary_for_its_parent_details() {
debug_assert!(self.as_element().local_name() == &local_name!("summary"));
// Step 1. If this summary element is not the summary for its parent details, then return.
if !self.is_a_summary_for_its_parent_details() {
return;
}
// Step 2
let parent_details = self.upcast::<Node>().GetParentNode().unwrap();
// Step 2. Let parent be this summary element's parent.
let parent = if self.is_implicit_summary_element() {
DomRoot::downcast::<HTMLDetailsElement>(self.containing_shadow_root().unwrap().Host())
.unwrap()
} else {
self.upcast::<Node>()
.GetParentNode()
.and_then(DomRoot::downcast::<HTMLDetailsElement>)
.unwrap()
};
// Step 3
parent_details
.downcast::<HTMLDetailsElement>()
.unwrap()
.toggle();
// Step 3. If the open attribute is present on parent, then remove it.
// Otherwise, set parent's open attribute to the empty string.
parent.toggle();
}
// https://html.spec.whatwg.org/multipage/#summary-for-its-parent-details
fn is_summary_for_its_parent_details(&self) -> bool {
// Step 1
let summary_node = self.upcast::<Node>();
if !summary_node.has_parent() {
/// <https://html.spec.whatwg.org/multipage/#summary-for-its-parent-details>
fn is_a_summary_for_its_parent_details(&self) -> bool {
if self.is_implicit_summary_element() {
return true;
}
// Step 1. If this summary element has no parent, then return false.
// Step 2. Let parent be this summary element's parent.
let Some(parent) = self.upcast::<Node>().GetParentNode() else {
return false;
}
};
// Step 2
let parent = &summary_node.GetParentNode().unwrap();
// Step 3
if !parent.is::<HTMLDetailsElement>() {
// Step 3. If parent is not a details element, then return false.
let Some(details) = parent.downcast::<HTMLDetailsElement>() else {
return false;
}
};
// Step 4 & 5
let first_summary_element = parent
.child_elements()
.find(|el| el.local_name() == &local_name!("summary"));
match first_summary_element {
Some(first_summary) => &*first_summary == self.as_element(),
None => false,
}
// Step 4. If parent's first summary element child is not this summary
// element, then return false.
// Step 5. Return true.
details
.find_corresponding_summary_element()
.is_some_and(|summary| &*summary == self.upcast())
}
/// Whether or not this is an implicitly generated `<summary>`
/// element for a UA `<details>` shadow tree
fn is_implicit_summary_element(&self) -> bool {
// Note that non-implicit summary elements are not actually inside
// the UA shadow tree, they're only assigned to a slot inside it.
// Therefore they don't cause false positives here
self.containing_shadow_root()
.as_deref()
.map(ShadowRoot::Host)
.is_some_and(|host| host.is::<HTMLDetailsElement>())
}
/// <https://html.spec.whatwg.org/multipage/#rendered-text-fragment>
@ -1173,6 +1194,7 @@ impl Activatable for HTMLElement {
self.summary_activation_behavior();
}
}
// Form-associated custom elements are the same interface type as
// normal HTMLElements, so HTMLElement needs to have the FormControl trait
// even though it's usually more specific trait implementations, like the

View file

@ -327,4 +327,17 @@ meter:-moz-meter-sub-optimum div {
}
meter:-moz-meter-sub-sub-optimum div {
background: linear-gradient(#f77, #f77, #fcc 20%, #d44 45%, #d44 55%);
}
/* https://html.spec.whatwg.org/#the-details-and-summary-elements */
details, summary {
display: block;
}
details > summary:first-of-type {
display: list-item;
counter-increment: list-item 0;
list-style: disclosure-closed inside;
}
details[open] > summary:first-of-type {
list-style-type: disclosure-open;
}

View file

@ -1,2 +0,0 @@
[details-open-pseudo-003.html]
expected: FAIL

View file

@ -25,3 +25,6 @@
[::first-line styles applied ("<div class='first-line-uppercase'>abc")]
expected: FAIL
[opened <details> content shown ("<div><details open><summary>abc</summary>123")]
expected: FAIL

View file

@ -1,6 +1,3 @@
[details-display.html]
[default display of first summary child of details is list-item]
expected: FAIL
[display of details element can be changed]
expected: FAIL

View file

@ -1,2 +0,0 @@
[details-revert.html]
expected: FAIL

View file

@ -0,0 +1,2 @@
[details-summary-display-inline-001.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[details-summary-display-inline-002.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[summary-display-list-item-001.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[summary-text-decoration.html]
expected: FAIL

View file

@ -0,0 +1,3 @@
[closed-details-layout-apis.tentative.html]
[Verifies the layout results of elements inside a closed <details> based on the usage of content-visibility:hidden.]
expected: FAIL

View file

@ -1,2 +0,0 @@
[details-add-summary.html]
expected: FAIL

View file

@ -7581,19 +7581,6 @@
{}
]
],
"duplicated_scroll_ids.html": [
"a0ac8e578ddb63efa9aa673285a38c67c4ba6c2b",
[
null,
[
[
"/_mozilla/mozilla/duplicated_scroll_ids_ref.html",
"=="
]
],
{}
]
],
"font-element-comma-separated.html": [
"db7e13f2ca3db0ebd3c610c3c25b052749b85e30",
[
@ -10463,10 +10450,6 @@
"3e68ff395f5475e2b618147f270117f576a5b7bd",
[]
],
"duplicated_scroll_ids_ref.html": [
"6783d72a6629f4938df8126dc5114d936eaaa48f",
[]
],
"font-element-comma-separated-ref.html": [
"97efe2b83d5f78bdac0d4aa951b63342fb1fa1cf",
[]

View file

@ -1,13 +0,0 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<link rel="match" href="duplicated_scroll_ids_ref.html">
<title>Ensure that content which produces duplicate scroll ids does not panic</title>
</head>
<body>
<div style="width: 100px; height: 100px; background: green"></div>
<details open style="overflow: auto">
</body>
</html>

View file

@ -1,11 +0,0 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Ensure that content which produces duplicate scroll ids does not panic</title>
</head>
<body>
<div style="width: 100px; height: 100px; background: green"></div>
</body>
</html>