From d7ec3635e60842c87f1565b4db6a8665bbc4a50a Mon Sep 17 00:00:00 2001 From: stevennovaryo Date: Tue, 6 May 2025 14:36:04 +0800 Subject: [PATCH 1/5] Use resources stylesheet for details element. Signed-off-by: stevennovaryo --- Cargo.lock | 1 + components/layout/layout_impl.rs | 27 +------ components/script/dom/htmldetailselement.rs | 78 +++++++++++---------- components/shared/embedder/resources.rs | 6 ++ components/shared/script_layout/Cargo.toml | 1 + components/shared/script_layout/lib.rs | 47 ++++++++++++- resources/details.css | 18 +++++ 7 files changed, 114 insertions(+), 64 deletions(-) create mode 100644 resources/details.css diff --git a/Cargo.lock b/Cargo.lock index cfc9a0a0abb..0ce8ddebcac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6551,6 +6551,7 @@ dependencies = [ "servo_malloc_size_of", "servo_url", "stylo", + "url", "webrender_api", ] diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 54edc215389..7c8ca09b024 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -36,7 +36,7 @@ use rayon::ThreadPool; use script::layout_dom::{ServoLayoutDocument, ServoLayoutElement, ServoLayoutNode}; use script_layout_interface::{ Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse, ReflowGoal, - ReflowRequest, ReflowResult, TrustedNodeAddress, + ReflowRequest, ReflowResult, TrustedNodeAddress, parse_ua_stylesheet, }; use script_traits::{DrawAPaintImageResult, PaintWorkletError, Painter, ScriptThreadMessage}; use servo_arc::Arc as ServoArc; @@ -58,7 +58,7 @@ use style::properties::{ComputedValues, PropertyId}; use style::queries::values::PrefersColorScheme; use style::selector_parser::{PseudoElement, RestyleDamage, SnapshotMap}; use style::servo::media_queries::FontMetricsProvider; -use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards}; +use style::shared_lock::{SharedRwLockReadGuard, StylesheetGuards}; use style::stylesheets::{ DocumentStyleSheet, Origin, Stylesheet, StylesheetInDocument, UrlExtraData, UserAgentStylesheets, @@ -72,7 +72,6 @@ use style::values::specified::font::{KeywordInfo, QueryFontMetricsFlags}; use style::{Zero, driver}; use style_traits::{CSSPixel, SpeculativePainter}; use stylo_atoms::Atom; -use url::Url; use webrender_api::units::{DevicePixel, DevicePoint, LayoutPixel, LayoutPoint, LayoutSize}; use webrender_api::{ExternalScrollId, HitTestFlags}; @@ -989,28 +988,6 @@ impl LayoutThread { } fn get_ua_stylesheets() -> Result { - fn parse_ua_stylesheet( - shared_lock: &SharedRwLock, - filename: &str, - content: &[u8], - ) -> Result { - let url = Url::parse(&format!("chrome://resources/{:?}", filename)) - .ok() - .unwrap(); - Ok(DocumentStyleSheet(ServoArc::new(Stylesheet::from_bytes( - content, - url.into(), - None, - None, - Origin::UserAgent, - MediaList::empty(), - shared_lock.clone(), - None, - None, - QuirksMode::NoQuirks, - )))) - } - let shared_lock = &GLOBAL_STYLE_DATA.shared_lock; // FIXME: presentational-hints.css should be at author origin with zero specificity. diff --git a/components/script/dom/htmldetailselement.rs b/components/script/dom/htmldetailselement.rs index 1d48b8e7a97..d6dacac911b 100644 --- a/components/script/dom/htmldetailselement.rs +++ b/components/script/dom/htmldetailselement.rs @@ -5,9 +5,14 @@ use std::cell::{Cell, Ref}; use dom_struct::dom_struct; +use embedder_traits::resources::Resource; use html5ever::{LocalName, Prefix, local_name}; use js::rust::HandleObject; +use script_layout_interface::parse_resource_stylesheet; +use style::attr::AttrValue; +use super::element::ElementCreator; +use super::types::HTMLLinkElement; use crate::dom::attr::Attr; use crate::dom::bindings::cell::DomRefCell; use crate::dom::bindings::codegen::Bindings::HTMLDetailsElementBinding::HTMLDetailsElementMethods; @@ -116,13 +121,49 @@ impl HTMLDetailsElement { ) .expect("Attaching UA shadow root failed"); + let link_element = HTMLLinkElement::new( + local_name!("link"), + None, + &document, + None, + ElementCreator::ScriptCreated, + can_gc, + ); + link_element.upcast::().set_attribute( + &local_name!("rel"), + AttrValue::String("stylesheet".to_owned()), + can_gc, + ); + root.upcast::() + .AppendChild(link_element.upcast::(), can_gc) + .unwrap(); + + let details_stylesheet = parse_resource_stylesheet( + link_element + .upcast::() + .owner_doc() + .style_shared_lock(), + Resource::DetailsCSS, + ); + link_element.set_stylesheet(details_stylesheet.unwrap()); + let summary = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); + summary.upcast::().set_attribute( + &local_name!("name"), + AttrValue::from_atomic("internal-main-summary".to_owned()), + can_gc, + ); root.upcast::() .AppendChild(summary.upcast::(), can_gc) .unwrap(); let fallback_summary = HTMLElement::new(local_name!("summary"), None, &document, None, can_gc); + fallback_summary.upcast::().set_attribute( + &local_name!("name"), + AttrValue::from_atomic("internal-fallback-summary".to_owned()), + can_gc, + ); fallback_summary .upcast::() .SetTextContent(Some(DEFAULT_SUMMARY.into()), can_gc); @@ -179,40 +220,6 @@ impl HTMLDetailsElement { } shadow_tree.descendants.Assign(slottable_children); } - - 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::() - .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::() - .set_string_attribute(&local_name!("style"), implicit_summary_style.into(), can_gc); - } } impl HTMLDetailsElementMethods for HTMLDetailsElement { @@ -234,8 +241,6 @@ impl VirtualMethods for HTMLDetailsElement { .attribute_mutated(attr, mutation, can_gc); if attr.local_name() == &local_name!("open") { - self.update_shadow_tree_styles(can_gc); - let counter = self.toggle_counter.get() + 1; self.toggle_counter.set(counter); @@ -263,6 +268,5 @@ impl VirtualMethods for HTMLDetailsElement { self.super_type().unwrap().bind_to_tree(context, can_gc); self.update_shadow_tree_contents(CanGc::note()); - self.update_shadow_tree_styles(CanGc::note()); } } diff --git a/components/shared/embedder/resources.rs b/components/shared/embedder/resources.rs index 28464a95d1a..7716fd20c92 100644 --- a/components/shared/embedder/resources.rs +++ b/components/shared/embedder/resources.rs @@ -66,6 +66,7 @@ pub fn sandbox_access_files_dirs() -> Vec { .unwrap_or_default() } +#[derive(Clone)] pub enum Resource { /// A list of GATT services that are blocked from being used by web bluetooth. /// The format of the file is a list of UUIDs, one per line, with an optional second word to specify the @@ -109,6 +110,9 @@ pub enum Resource { DirectoryListingHTML, /// A HTML page that is used for the about:memory url. AboutMemoryHTML, + /// A CSS file to style the elements inside
element UA Shadow Tree. + /// It can be empty but then
element simply wouldn't work. + DetailsCSS, } impl Resource { @@ -123,6 +127,7 @@ impl Resource { Resource::CrashHTML => "crash.html", Resource::DirectoryListingHTML => "directory-listing.html", Resource::AboutMemoryHTML => "about-memory.html", + Resource::DetailsCSS => "details.css", } } } @@ -167,6 +172,7 @@ fn resources_for_tests() -> Box { Resource::AboutMemoryHTML => { &include_bytes!("../../../resources/about-memory.html")[..] }, + Resource::DetailsCSS => &include_bytes!("../../../resources/details.css")[..], } .to_owned() } diff --git a/components/shared/script_layout/Cargo.toml b/components/shared/script_layout/Cargo.toml index 167606f7247..e11ede50155 100644 --- a/components/shared/script_layout/Cargo.toml +++ b/components/shared/script_layout/Cargo.toml @@ -38,4 +38,5 @@ serde = { workspace = true } servo_arc = { workspace = true } servo_url = { path = "../../url" } stylo = { workspace = true } +url = { workspace = true } webrender_api = { workspace = true } diff --git a/components/shared/script_layout/lib.rs b/components/shared/script_layout/lib.rs index e82ee16e24b..ac521c622c9 100644 --- a/components/shared/script_layout/lib.rs +++ b/components/shared/script_layout/lib.rs @@ -20,6 +20,7 @@ use base::Epoch; use base::id::{BrowsingContextId, PipelineId, WebViewId}; use compositing_traits::CrossProcessCompositorApi; use constellation_traits::{LoadData, ScrollState}; +use embedder_traits::resources::{self, Resource}; use embedder_traits::{Theme, UntrustedNodeAddress, ViewportDetails}; use euclid::default::{Point2D, Rect}; use fnv::FnvHashMap; @@ -43,11 +44,13 @@ use style::context::QuirksMode; use style::data::ElementData; use style::dom::OpaqueNode; use style::invalidation::element::restyle_hints::RestyleHint; -use style::media_queries::Device; +use style::media_queries::{Device, MediaList}; use style::properties::PropertyId; use style::properties::style_structs::Font; use style::selector_parser::{PseudoElement, RestyleDamage, Snapshot}; -use style::stylesheets::Stylesheet; +use style::shared_lock::SharedRwLock; +use style::stylesheets::{DocumentStyleSheet, Origin, Stylesheet}; +use url::Url; use webrender_api::ImageKey; use webrender_api::units::DeviceIntSize; @@ -635,3 +638,43 @@ mod test { assert_eq!(image_animation_state.last_update_time, 0.101); } } + +pub fn parse_stylesheet_as_origin( + shared_lock: &SharedRwLock, + filename: &str, + content: &[u8], + origin: Origin, +) -> Result, &'static str> { + let url = Url::parse(&format!("chrome://resources/{:?}", filename)) + .ok() + .unwrap(); + Ok(ServoArc::new(Stylesheet::from_bytes( + content, + url.into(), + None, + None, + origin, + MediaList::empty(), + shared_lock.clone(), + None, + None, + QuirksMode::NoQuirks, + ))) +} + +pub fn parse_ua_stylesheet( + shared_lock: &SharedRwLock, + filename: &str, + content: &[u8], +) -> Result { + parse_stylesheet_as_origin(shared_lock, filename, content, Origin::UserAgent) + .map(DocumentStyleSheet) +} + +pub fn parse_resource_stylesheet( + shared_lock: &SharedRwLock, + resources: Resource, +) -> Result, &'static str> { + let content = &resources::read_bytes(resources.clone()); + parse_stylesheet_as_origin(shared_lock, resources.filename(), content, Origin::Author) +} diff --git a/resources/details.css b/resources/details.css new file mode 100644 index 00000000000..81331ae22c2 --- /dev/null +++ b/resources/details.css @@ -0,0 +1,18 @@ +/* We should use `content-visibility` but it is yet to be implemented. + * These rule would not comply with ::details-content and should be removed + * with it's implementation */ +slot:not([name]) { + display: none; +} +:host([open]) slot:not([name]) { + display: block; +} + +summary[name=internal-fallback-summary] { + display: list-item; + counter-increment: list-item 0; + list-style: disclosure-closed inside; +} +:host([open]) summary[name=internal-fallback-summary] { + list-style-type: disclosure-open; +} From 3da5ef2cd42ddfcc32918cf04dcbeb116687a260 Mon Sep 17 00:00:00 2001 From: stevennovaryo Date: Tue, 13 May 2025 21:37:39 +0800 Subject: [PATCH 2/5] Embed details stylesheet Signed-off-by: stevennovaryo --- .../layout/stylesheets}/details.css | 0 components/script/dom/htmldetailselement.rs | 6 ++---- components/shared/embedder/resources.rs | 6 ------ components/shared/script_layout/lib.rs | 10 +++++----- 4 files changed, 7 insertions(+), 15 deletions(-) rename {resources => components/layout/stylesheets}/details.css (100%) diff --git a/resources/details.css b/components/layout/stylesheets/details.css similarity index 100% rename from resources/details.css rename to components/layout/stylesheets/details.css diff --git a/components/script/dom/htmldetailselement.rs b/components/script/dom/htmldetailselement.rs index d6dacac911b..1db5d035c94 100644 --- a/components/script/dom/htmldetailselement.rs +++ b/components/script/dom/htmldetailselement.rs @@ -5,10 +5,9 @@ use std::cell::{Cell, Ref}; use dom_struct::dom_struct; -use embedder_traits::resources::Resource; use html5ever::{LocalName, Prefix, local_name}; use js::rust::HandleObject; -use script_layout_interface::parse_resource_stylesheet; +use script_layout_interface::parse_details_stylesheet; use style::attr::AttrValue; use super::element::ElementCreator; @@ -138,12 +137,11 @@ impl HTMLDetailsElement { .AppendChild(link_element.upcast::(), can_gc) .unwrap(); - let details_stylesheet = parse_resource_stylesheet( + let details_stylesheet = parse_details_stylesheet( link_element .upcast::() .owner_doc() .style_shared_lock(), - Resource::DetailsCSS, ); link_element.set_stylesheet(details_stylesheet.unwrap()); diff --git a/components/shared/embedder/resources.rs b/components/shared/embedder/resources.rs index 7716fd20c92..28464a95d1a 100644 --- a/components/shared/embedder/resources.rs +++ b/components/shared/embedder/resources.rs @@ -66,7 +66,6 @@ pub fn sandbox_access_files_dirs() -> Vec { .unwrap_or_default() } -#[derive(Clone)] pub enum Resource { /// A list of GATT services that are blocked from being used by web bluetooth. /// The format of the file is a list of UUIDs, one per line, with an optional second word to specify the @@ -110,9 +109,6 @@ pub enum Resource { DirectoryListingHTML, /// A HTML page that is used for the about:memory url. AboutMemoryHTML, - /// A CSS file to style the elements inside
element UA Shadow Tree. - /// It can be empty but then
element simply wouldn't work. - DetailsCSS, } impl Resource { @@ -127,7 +123,6 @@ impl Resource { Resource::CrashHTML => "crash.html", Resource::DirectoryListingHTML => "directory-listing.html", Resource::AboutMemoryHTML => "about-memory.html", - Resource::DetailsCSS => "details.css", } } } @@ -172,7 +167,6 @@ fn resources_for_tests() -> Box { Resource::AboutMemoryHTML => { &include_bytes!("../../../resources/about-memory.html")[..] }, - Resource::DetailsCSS => &include_bytes!("../../../resources/details.css")[..], } .to_owned() } diff --git a/components/shared/script_layout/lib.rs b/components/shared/script_layout/lib.rs index ac521c622c9..37d05c31286 100644 --- a/components/shared/script_layout/lib.rs +++ b/components/shared/script_layout/lib.rs @@ -20,7 +20,6 @@ use base::Epoch; use base::id::{BrowsingContextId, PipelineId, WebViewId}; use compositing_traits::CrossProcessCompositorApi; use constellation_traits::{LoadData, ScrollState}; -use embedder_traits::resources::{self, Resource}; use embedder_traits::{Theme, UntrustedNodeAddress, ViewportDetails}; use euclid::default::{Point2D, Rect}; use fnv::FnvHashMap; @@ -54,6 +53,9 @@ use url::Url; use webrender_api::ImageKey; use webrender_api::units::DeviceIntSize; +/// A CSS file to style
element. +static DETAILS_CSS: &[u8] = include_bytes!("../../layout/stylesheets/details.css"); + pub trait GenericLayoutDataTrait: Any + MallocSizeOfTrait { fn as_any(&self) -> &dyn Any; } @@ -671,10 +673,8 @@ pub fn parse_ua_stylesheet( .map(DocumentStyleSheet) } -pub fn parse_resource_stylesheet( +pub fn parse_details_stylesheet( shared_lock: &SharedRwLock, - resources: Resource, ) -> Result, &'static str> { - let content = &resources::read_bytes(resources.clone()); - parse_stylesheet_as_origin(shared_lock, resources.filename(), content, Origin::Author) + parse_stylesheet_as_origin(shared_lock, "details.css", DETAILS_CSS, Origin::Author) } From a824b2781f93306d2e985177df7097f5f8ff2bca Mon Sep 17 00:00:00 2001 From: stevennovaryo Date: Thu, 15 May 2025 15:04:59 +0800 Subject: [PATCH 3/5] Use id instead of name for matching Signed-off-by: stevennovaryo --- components/layout/stylesheets/details.css | 8 +++--- components/script/dom/htmldetailselement.rs | 31 +++++++++++---------- components/shared/script_layout/lib.rs | 8 ++++++ 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/components/layout/stylesheets/details.css b/components/layout/stylesheets/details.css index 81331ae22c2..db3ccf3de47 100644 --- a/components/layout/stylesheets/details.css +++ b/components/layout/stylesheets/details.css @@ -1,18 +1,18 @@ /* We should use `content-visibility` but it is yet to be implemented. * These rule would not comply with ::details-content and should be removed * with it's implementation */ -slot:not([name]) { +slot[id="internal-contents-slot"] { display: none; } -:host([open]) slot:not([name]) { +:host([open]) slot[id="internal-contents-slot"] { display: block; } -summary[name=internal-fallback-summary] { +summary[id="internal-fallback-summary"] { display: list-item; counter-increment: list-item 0; list-style: disclosure-closed inside; } -:host([open]) summary[name=internal-fallback-summary] { +:host([open]) summary[id="internal-fallback-summary"] { list-style-type: disclosure-open; } diff --git a/components/script/dom/htmldetailselement.rs b/components/script/dom/htmldetailselement.rs index 1db5d035c94..9e1b18c3b5f 100644 --- a/components/script/dom/htmldetailselement.rs +++ b/components/script/dom/htmldetailselement.rs @@ -47,8 +47,6 @@ const DEFAULT_SUMMARY: &str = "Details"; struct ShadowTree { summary: Dom, descendants: Dom, - /// The summary that is displayed if no other summary exists - implicit_summary: Dom, } #[dom_struct] @@ -145,40 +143,45 @@ impl HTMLDetailsElement { ); link_element.set_stylesheet(details_stylesheet.unwrap()); - let summary = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); - summary.upcast::().set_attribute( - &local_name!("name"), - AttrValue::from_atomic("internal-main-summary".to_owned()), + let summary_slot = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); + summary_slot.upcast::().set_attribute( + &local_name!("id"), + AttrValue::from_atomic("internal-summary-slot".to_owned()), can_gc, ); root.upcast::() - .AppendChild(summary.upcast::(), can_gc) + .AppendChild(summary_slot.upcast::(), can_gc) .unwrap(); let fallback_summary = HTMLElement::new(local_name!("summary"), None, &document, None, can_gc); fallback_summary.upcast::().set_attribute( - &local_name!("name"), + &local_name!("id"), AttrValue::from_atomic("internal-fallback-summary".to_owned()), can_gc, ); fallback_summary .upcast::() .SetTextContent(Some(DEFAULT_SUMMARY.into()), can_gc); - summary + summary_slot .upcast::() .AppendChild(fallback_summary.upcast::(), can_gc) .unwrap(); - let descendants = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); + let descendants_slot = + HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); + descendants_slot.upcast::().set_attribute( + &local_name!("id"), + AttrValue::from_atomic("internal-contents-slot".to_owned()), + can_gc, + ); root.upcast::() - .AppendChild(descendants.upcast::(), can_gc) + .AppendChild(descendants_slot.upcast::(), can_gc) .unwrap(); let _ = self.shadow_tree.borrow_mut().insert(ShadowTree { - summary: summary.as_traced(), - descendants: descendants.as_traced(), - implicit_summary: fallback_summary.as_traced(), + summary: summary_slot.as_traced(), + descendants: descendants_slot.as_traced(), }); self.upcast::() .dirty(crate::dom::node::NodeDamage::OtherNodeDamage); diff --git a/components/shared/script_layout/lib.rs b/components/shared/script_layout/lib.rs index 37d05c31286..94bcd910915 100644 --- a/components/shared/script_layout/lib.rs +++ b/components/shared/script_layout/lib.rs @@ -673,8 +673,16 @@ pub fn parse_ua_stylesheet( .map(DocumentStyleSheet) } +/// Parse stylesheet for
element to insert its UA shadow DOM. +/// +/// TODO(stevennovaryo): The more approriate way to handle this is to use UA stylesheet. +/// But this element's styles needs to be in shadow-scoped stylesheet. +/// This could be done if an UA shadow-scoped UA sheet is introduced. pub fn parse_details_stylesheet( shared_lock: &SharedRwLock, ) -> Result, &'static str> { + // FIXME: We are parsing it as a Author stylesheet, but according to, it's nature + // it should be an user agent stylesheet. This is because we are only allowing + // the actual UA stylesheet to have that origin. parse_stylesheet_as_origin(shared_lock, "details.css", DETAILS_CSS, Origin::Author) } From 9d469fc917b70f365c0923fdad68efa620308952 Mon Sep 17 00:00:00 2001 From: stevennovaryo Date: Fri, 23 May 2025 16:53:54 +0800 Subject: [PATCH 4/5] Fix details children slotting and apply summary Signed-off-by: stevennovaryo --- components/layout/stylesheets/details.css | 4 +- components/script/dom/htmldetailselement.rs | 119 ++++++++++++-------- 2 files changed, 74 insertions(+), 49 deletions(-) diff --git a/components/layout/stylesheets/details.css b/components/layout/stylesheets/details.css index db3ccf3de47..d7ff253f501 100644 --- a/components/layout/stylesheets/details.css +++ b/components/layout/stylesheets/details.css @@ -8,11 +8,11 @@ slot[id="internal-contents-slot"] { display: block; } -summary[id="internal-fallback-summary"] { +:host summary:first-of-type { display: list-item; counter-increment: list-item 0; list-style: disclosure-closed inside; } -:host([open]) summary[id="internal-fallback-summary"] { +:host([open]) summary:first-of-type { list-style-type: disclosure-open; } diff --git a/components/script/dom/htmldetailselement.rs b/components/script/dom/htmldetailselement.rs index 9e1b18c3b5f..c57fec5ed6e 100644 --- a/components/script/dom/htmldetailselement.rs +++ b/components/script/dom/htmldetailselement.rs @@ -45,8 +45,9 @@ const DEFAULT_SUMMARY: &str = "Details"; #[derive(Clone, JSTraceable, MallocSizeOf)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] struct ShadowTree { - summary: Dom, - descendants: Dom, + summary_slot: Dom, + descendants_slot: Dom, + fallback_summary: Dom, } #[dom_struct] @@ -103,6 +104,7 @@ impl HTMLDetailsElement { .expect("UA shadow tree was not created") } + /// fn create_shadow_tree(&self, can_gc: CanGc) { let document = self.owner_document(); let root = self @@ -118,6 +120,50 @@ impl HTMLDetailsElement { ) .expect("Attaching UA shadow root failed"); + // > The details element is expected to have an internal shadow tree with three child elements: + // > 1. The first child element is a slot that is expected to take the details element's first summary + // > element child, if any. + let summary_slot = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); + summary_slot.upcast::().set_attribute( + &local_name!("id"), + AttrValue::from_atomic("internal-summary-slot".to_owned()), + can_gc, + ); + root.upcast::() + .AppendChild(summary_slot.upcast::(), can_gc) + .unwrap(); + + // > This element has a single child summary element called the default summary which has + // > text content that is implementation-defined (and probably locale-specific). + let fallback_summary = + HTMLElement::new(local_name!("summary"), None, &document, None, can_gc); + fallback_summary.upcast::().set_attribute( + &local_name!("id"), + AttrValue::from_atomic("internal-fallback-summary".to_owned()), + can_gc, + ); + fallback_summary + .upcast::() + .SetTextContent(Some(DEFAULT_SUMMARY.into()), can_gc); + summary_slot + .upcast::() + .AppendChild(fallback_summary.upcast::(), can_gc) + .unwrap(); + + // > 2. The second child element is a slot that is expected to take the details element's + // > remaining descendants, if any. This element has no contents. + let descendants_slot = + HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); + descendants_slot.upcast::().set_attribute( + &local_name!("id"), + AttrValue::from_atomic("internal-contents-slot".to_owned()), + can_gc, + ); + root.upcast::() + .AppendChild(descendants_slot.upcast::(), can_gc) + .unwrap(); + + // > 3. The third child element is either a link or style element with the following styles for the default summary: let link_element = HTMLLinkElement::new( local_name!("link"), None, @@ -135,6 +181,8 @@ impl HTMLDetailsElement { .AppendChild(link_element.upcast::(), can_gc) .unwrap(); + // TODO(stevennovaryo): We need a mechanism for parsing and storing a stylesheet as + // we shouldn't reparse this each time we access the element. let details_stylesheet = parse_details_stylesheet( link_element .upcast::() @@ -143,45 +191,11 @@ impl HTMLDetailsElement { ); link_element.set_stylesheet(details_stylesheet.unwrap()); - let summary_slot = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); - summary_slot.upcast::().set_attribute( - &local_name!("id"), - AttrValue::from_atomic("internal-summary-slot".to_owned()), - can_gc, - ); - root.upcast::() - .AppendChild(summary_slot.upcast::(), can_gc) - .unwrap(); - - let fallback_summary = - HTMLElement::new(local_name!("summary"), None, &document, None, can_gc); - fallback_summary.upcast::().set_attribute( - &local_name!("id"), - AttrValue::from_atomic("internal-fallback-summary".to_owned()), - can_gc, - ); - fallback_summary - .upcast::() - .SetTextContent(Some(DEFAULT_SUMMARY.into()), can_gc); - summary_slot - .upcast::() - .AppendChild(fallback_summary.upcast::(), can_gc) - .unwrap(); - - let descendants_slot = - HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc); - descendants_slot.upcast::().set_attribute( - &local_name!("id"), - AttrValue::from_atomic("internal-contents-slot".to_owned()), - can_gc, - ); - root.upcast::() - .AppendChild(descendants_slot.upcast::(), can_gc) - .unwrap(); let _ = self.shadow_tree.borrow_mut().insert(ShadowTree { - summary: summary_slot.as_traced(), - descendants: descendants_slot.as_traced(), + summary_slot: summary_slot.as_traced(), + descendants_slot: descendants_slot.as_traced(), + fallback_summary: fallback_summary.as_traced(), }); self.upcast::() .dirty(crate::dom::node::NodeDamage::OtherNodeDamage); @@ -199,16 +213,18 @@ impl HTMLDetailsElement { 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 discovered_first_summary_element = false; let mut slottable_children = vec![]; + + // Assign all children to its corresponding slots. for child in self.upcast::().children() { if let Some(element) = child.downcast::() { - if element.local_name() == &local_name!("summary") { + // Assign the first summary element to the summary slot. + if element.local_name() == &local_name!("summary") && !discovered_first_summary_element { + shadow_tree + .summary_slot + .Assign(vec![ElementOrText::Element(DomRoot::from_ref(element))]); + discovered_first_summary_element = true; continue; } @@ -219,7 +235,16 @@ impl HTMLDetailsElement { slottable_children.push(ElementOrText::Text(DomRoot::from_ref(text))); } } - shadow_tree.descendants.Assign(slottable_children); + shadow_tree.descendants_slot.Assign(slottable_children); + + // If there are no summary element, assign fallback summary instead. + if !discovered_first_summary_element { + shadow_tree + .summary_slot + .Assign(vec![ElementOrText::Element(DomRoot::upcast( + shadow_tree.fallback_summary.as_rooted() + ))]); + } } } From 8ede3509cab60f7d8989c49e6c2848c0e7aab499 Mon Sep 17 00:00:00 2001 From: stevennovaryo Date: Fri, 30 May 2025 01:21:16 +0800 Subject: [PATCH 5/5] Style nit Signed-off-by: stevennovaryo --- components/layout/stylesheets/details.css | 4 +-- components/script/dom/htmldetailselement.rs | 12 ++++---- tests/wpt/meta/MANIFEST.json | 30 +++++++++++++++++++ .../details-content-security-policy.html | 14 +++++++++ .../details-multiple-summaries-ref.html | 19 ++++++++++++ .../details-multiple-summaries.html | 15 ++++++++++ 6 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 tests/wpt/tests/html/rendering/the-details-element/details-content-security-policy.html create mode 100644 tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries-ref.html create mode 100644 tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries.html diff --git a/components/layout/stylesheets/details.css b/components/layout/stylesheets/details.css index d7ff253f501..5dc63815391 100644 --- a/components/layout/stylesheets/details.css +++ b/components/layout/stylesheets/details.css @@ -1,10 +1,10 @@ /* We should use `content-visibility` but it is yet to be implemented. * These rule would not comply with ::details-content and should be removed * with it's implementation */ -slot[id="internal-contents-slot"] { +slot#internal-contents-slot { display: none; } -:host([open]) slot[id="internal-contents-slot"] { +:host([open]) slot#internal-contents-slot { display: block; } diff --git a/components/script/dom/htmldetailselement.rs b/components/script/dom/htmldetailselement.rs index c57fec5ed6e..423bfe45d97 100644 --- a/components/script/dom/htmldetailselement.rs +++ b/components/script/dom/htmldetailselement.rs @@ -104,7 +104,7 @@ impl HTMLDetailsElement { .expect("UA shadow tree was not created") } - /// + /// fn create_shadow_tree(&self, can_gc: CanGc) { let document = self.owner_document(); let root = self @@ -163,7 +163,8 @@ impl HTMLDetailsElement { .AppendChild(descendants_slot.upcast::(), can_gc) .unwrap(); - // > 3. The third child element is either a link or style element with the following styles for the default summary: + // > 3. The third child element is either a link or style element with the following + // > styles for the default summary: let link_element = HTMLLinkElement::new( local_name!("link"), None, @@ -191,7 +192,6 @@ impl HTMLDetailsElement { ); link_element.set_stylesheet(details_stylesheet.unwrap()); - let _ = self.shadow_tree.borrow_mut().insert(ShadowTree { summary_slot: summary_slot.as_traced(), descendants_slot: descendants_slot.as_traced(), @@ -220,7 +220,9 @@ impl HTMLDetailsElement { for child in self.upcast::().children() { if let Some(element) = child.downcast::() { // Assign the first summary element to the summary slot. - if element.local_name() == &local_name!("summary") && !discovered_first_summary_element { + if element.local_name() == &local_name!("summary") && + !discovered_first_summary_element + { shadow_tree .summary_slot .Assign(vec![ElementOrText::Element(DomRoot::from_ref(element))]); @@ -242,7 +244,7 @@ impl HTMLDetailsElement { shadow_tree .summary_slot .Assign(vec![ElementOrText::Element(DomRoot::upcast( - shadow_tree.fallback_summary.as_rooted() + shadow_tree.fallback_summary.as_rooted(), ))]); } } diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 0e367d5653e..b44f105f9ef 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -352512,6 +352512,19 @@ {} ] ], + "details-content-security-policy.html": [ + "2e98d81e31420d43dcdff110dd3703c5a2e9ddb4", + [ + "html/rendering/the-details-element/details-content-security-policy.html", + [ + [ + "/html/rendering/the-details-element/single-summary.html", + "==" + ] + ], + {} + ] + ], "details-display-type-001-ref.html": [ "925ee19f775fd07ac6e679598329d05f609e5335", [ @@ -352551,6 +352564,19 @@ {} ] ], + "details-multiple-summaries.html": [ + "55829ce57bc1cf12109ac2d553fa93a86b4422b4", + [ + "html/rendering/the-details-element/details-multiple-summaries.html", + [ + [ + "/html/rendering/the-details-element/details-multiple-summaries-ref.html", + "==" + ] + ], + {} + ] + ], "details-pseudo-elements-001.html": [ "f9d48101fb65edfc868a67a5722cdbfc0e8f7f1b", [ @@ -480162,6 +480188,10 @@ "297634b521fab5e61d0bbac9edb4480b5de7366d", [] ], + "details-multiple-summaries-ref.html": [ + "a23296880c15e3578f1428f0ca51073fc49df6c2", + [] + ], "details-pseudo-elements-001-ref.html": [ "0c77c0e14fc3e0f3707e6dee32aea5a9738b6fb1", [] diff --git a/tests/wpt/tests/html/rendering/the-details-element/details-content-security-policy.html b/tests/wpt/tests/html/rendering/the-details-element/details-content-security-policy.html new file mode 100644 index 00000000000..2e98d81e314 --- /dev/null +++ b/tests/wpt/tests/html/rendering/the-details-element/details-content-security-policy.html @@ -0,0 +1,14 @@ + + + + + + + Details Element Appearance Should not be Affected By Content Security Policy + + + +
+ This is the main summary + This is the content +
diff --git a/tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries-ref.html b/tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries-ref.html new file mode 100644 index 00000000000..a23296880c1 --- /dev/null +++ b/tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries-ref.html @@ -0,0 +1,19 @@ + + + + + Details Element Should not Discard the Additional Summary Element + + + + +
+ This is the main summary + This is the additional summary + This is the content +
diff --git a/tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries.html b/tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries.html new file mode 100644 index 00000000000..55829ce57bc --- /dev/null +++ b/tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries.html @@ -0,0 +1,15 @@ + + + + + Details Element Should not Discard the Additional Summary Element + + + + + +
+ This is the main summary + This is the additional summary + This is the content +