Merge 8ede3509ca into ad95a74389

Cargo.lock

@@ -6545,6 +6545,7 @@ dependencies = [
  "servo_malloc_size_of",
  "servo_url",
  "stylo",
+ "url",
  "webrender_api",
 ]
 

components/layout/layout_impl.rs

@@ -37,7 +37,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;
@@ -59,7 +59,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,
@@ -73,7 +73,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};
 
@@ -1009,28 +1008,6 @@ impl LayoutThread {
 }
 
 fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> {
-    fn parse_ua_stylesheet(
-        shared_lock: &SharedRwLock,
-        filename: &str,
-        content: &[u8],
-    ) -> Result<DocumentStyleSheet, &'static str> {
-        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.

components/layout/stylesheets/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#internal-contents-slot {
+  display: none;
+}
+:host([open]) slot#internal-contents-slot {
+  display: block;
+}
+
+:host summary:first-of-type {
+  display: list-item;
+  counter-increment: list-item 0;
+  list-style: disclosure-closed inside;
+}
+:host([open]) summary:first-of-type {
+  list-style-type: disclosure-open;
+}

components/script/dom/htmldetailselement.rs

@@ -7,7 +7,11 @@ use std::cell::{Cell, Ref};
 use dom_struct::dom_struct;
 use html5ever::{LocalName, Prefix, local_name};
 use js::rust::HandleObject;
+use script_layout_interface::parse_details_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;
@@ -41,10 +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<HTMLSlotElement>,
-    descendants: Dom<HTMLSlotElement>,
-    /// The summary that is displayed if no other summary exists
-    implicit_summary: Dom<HTMLElement>,
+    summary_slot: Dom<HTMLSlotElement>,
+    descendants_slot: Dom<HTMLSlotElement>,
+    fallback_summary: Dom<HTMLElement>,
 }
 
 #[dom_struct]
@@ -101,6 +104,7 @@ impl HTMLDetailsElement {
             .expect("UA shadow tree was not created")
     }
 
+    /// <https://html.spec.whatwg.org/multipage/#the-details-and-summary-elements>
     fn create_shadow_tree(&self, can_gc: CanGc) {
         let document = self.owner_document();
         let root = self
@@ -116,30 +120,82 @@ impl HTMLDetailsElement {
             )
             .expect("Attaching UA shadow root failed");
 
-        let summary = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc);
+        // > 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::<Element>().set_attribute(
+            &local_name!("id"),
+            AttrValue::from_atomic("internal-summary-slot".to_owned()),
+            can_gc,
+        );
         root.upcast::<Node>()
-            .AppendChild(summary.upcast::<Node>(), can_gc)
+            .AppendChild(summary_slot.upcast::<Node>(), 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::<Element>().set_attribute(
+            &local_name!("id"),
+            AttrValue::from_atomic("internal-fallback-summary".to_owned()),
+            can_gc,
+        );
         fallback_summary
             .upcast::<Node>()
             .SetTextContent(Some(DEFAULT_SUMMARY.into()), can_gc);
-        summary
+        summary_slot
             .upcast::<Node>()
             .AppendChild(fallback_summary.upcast::<Node>(), can_gc)
             .unwrap();
 
-        let descendants = HTMLSlotElement::new(local_name!("slot"), None, &document, None, can_gc);
+        // > 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::<Element>().set_attribute(
+            &local_name!("id"),
+            AttrValue::from_atomic("internal-contents-slot".to_owned()),
+            can_gc,
+        );
         root.upcast::<Node>()
-            .AppendChild(descendants.upcast::<Node>(), can_gc)
+            .AppendChild(descendants_slot.upcast::<Node>(), 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,
+            &document,
+            None,
+            ElementCreator::ScriptCreated,
+            can_gc,
+        );
+        link_element.upcast::<Element>().set_attribute(
+            &local_name!("rel"),
+            AttrValue::String("stylesheet".to_owned()),
+            can_gc,
+        );
+        root.upcast::<Node>()
+            .AppendChild(link_element.upcast::<Node>(), 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::<Node>()
+                .owner_doc()
+                .style_shared_lock(),
+        );
+        link_element.set_stylesheet(details_stylesheet.unwrap());
+
         let _ = self.shadow_tree.borrow_mut().insert(ShadowTree {
-            summary: summary.as_traced(),
-            descendants: descendants.as_traced(),
-            implicit_summary: fallback_summary.as_traced(),
+            summary_slot: summary_slot.as_traced(),
+            descendants_slot: descendants_slot.as_traced(),
+            fallback_summary: fallback_summary.as_traced(),
         });
         self.upcast::<Node>()
             .dirty(crate::dom::node::NodeDamage::OtherNodeDamage);
@@ -157,16 +213,20 @@ 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::<Node>().children() {
             if let Some(element) = child.downcast::<Element>() {
-                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;
                 }
 
@@ -177,41 +237,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);
 
-    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);
+        // 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(),
+                ))]);
+        }
     }
 }
 
@@ -234,8 +269,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 +296,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());
     }
 }

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 }

components/shared/script_layout/lib.rs

@@ -43,14 +43,19 @@ 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;
 
+/// A CSS file to style <details> element.
+static DETAILS_CSS: &[u8] = include_bytes!("../../layout/stylesheets/details.css");
+
 pub trait GenericLayoutDataTrait: Any + MallocSizeOfTrait {
     fn as_any(&self) -> &dyn Any;
 }
@@ -635,3 +640,49 @@ 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<ServoArc<Stylesheet>, &'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<DocumentStyleSheet, &'static str> {
+    parse_stylesheet_as_origin(shared_lock, filename, content, Origin::UserAgent)
+        .map(DocumentStyleSheet)
+}
+
+/// Parse stylesheet for <details> 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<ServoArc<Stylesheet>, &'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)
+}

tests/wpt/meta/MANIFEST.json

@@ -352951,6 +352951,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",
        [
@@ -352990,6 +353003,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",
        [
@@ -480710,6 +480736,10 @@
        "297634b521fab5e61d0bbac9edb4480b5de7366d",
        []
       ],
+      "details-multiple-summaries-ref.html": [
+       "a23296880c15e3578f1428f0ca51073fc49df6c2",
+       []
+      ],
       "details-pseudo-elements-001-ref.html": [
        "0c77c0e14fc3e0f3707e6dee32aea5a9738b6fb1",
        []

tests/wpt/tests/html/rendering/the-details-element/details-content-security-policy.html

@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <meta http-equiv="Content-Security-Policy" content="default-src 'none';">
+  <link rel="author" title="Jo Steven Novaryo" href="mailto:steven.novaryo@gmail.com">
+  <title>Details Element Appearance Should not be Affected By Content Security Policy</title>
+  <link rel="match" href="single-summary.html">
+</head>
+
+<details>
+  <summary>This is the main summary</summary>
+  This is the content
+</details>

tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries-ref.html

@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <title>Details Element Should not Discard the Additional Summary Element</title>
+</head>
+
+<style>
+  #main-summary {
+    display: list-item;
+    list-style: disclosure-open inside;
+  }
+</style>
+
+<div>
+  <summary id="main-summary">This is the main summary</summary>
+  <summary>This is the additional summary</summary>
+  This is the content
+</div>

tests/wpt/tests/html/rendering/the-details-element/details-multiple-summaries.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <title>Details Element Should not Discard the Additional Summary Element</title>
+  <link rel="help" href="https://html.spec.whatwg.org/multipage/rendering.html#the-details-and-summary-elements">
+  <link rel="match" href="details-multiple-summaries-ref.html">
+  <link rel="author" title="Jo Steven Novaryo" href="mailto:steven.novaryo@gmail.com">
+</head>
+
+<details open>
+  <summary>This is the main summary</summary>
+  <summary id="second-summary">This is the additional summary</summary>
+  This is the content
+</details>