diff --git a/crates/obscura-dom/src/selector.rs b/crates/obscura-dom/src/selector.rs index 4c5acf4..6f85797 100644 --- a/crates/obscura-dom/src/selector.rs +++ b/crates/obscura-dom/src/selector.rs @@ -580,16 +580,21 @@ impl DomTree { // Fast path: a bare "#id" selector resolves through the id index in O(1) // instead of scanning every descendant. The index holds the first element // in tree order per id, which is exactly what the full scan would return. - if let Some(id) = simple_id_selector(selector) { - match self.get_element_by_id(id) { - // querySelector matches strict descendants of root only, so the - // indexed element must have root among its ancestors. - Some(nid) if self.ancestors(nid).contains(&root) => return Ok(Some(nid)), - // Index miss or stale entry (detached node) — fall through to full scan. - // The id_index is best-effort: it only registers nodes at creation time - // and does not update on reparent, so it can point to a detached clone - // while the live node (with the same id) is elsewhere in the tree. - _ => {} + // In quirks mode `#id` matches ASCII-case-insensitively, but the id + // index is keyed on the exact-case id, so skip the fast path and let the + // selector engine (below) do the case-insensitive match. + if !self.is_quirks() { + if let Some(id) = simple_id_selector(selector) { + match self.get_element_by_id(id) { + // querySelector matches strict descendants of root only, so the + // indexed element must have root among its ancestors. + Some(nid) if self.ancestors(nid).contains(&root) => return Ok(Some(nid)), + // Index miss or stale entry (detached node) — fall through to full scan. + // The id_index is best-effort: it only registers nodes at creation time + // and does not update on reparent, so it can point to a detached clone + // while the live node (with the same id) is elsewhere in the tree. + _ => {} + } } } let selector_list = parse_selector(selector)?; @@ -598,7 +603,7 @@ impl DomTree { MatchingMode::Normal, None, &mut caches, - QuirksMode::NoQuirks, + self.selector_quirks_mode(), NeedsSelectorFlags::No, MatchingForInvalidation::No, ); @@ -619,6 +624,16 @@ impl DomTree { Ok(None) } + // Map the document's quirks flag onto the selector crate's QuirksMode. In + // quirks mode the crate matches class/id selectors ASCII-case-insensitively. + fn selector_quirks_mode(&self) -> QuirksMode { + if self.is_quirks() { + QuirksMode::Quirks + } else { + QuirksMode::NoQuirks + } + } + pub fn query_selector_all_from(&self, root: NodeId, selector: &str) -> Result, String> { let selector_list = parse_selector(selector)?; let mut caches = selectors::context::SelectorCaches::default(); @@ -626,7 +641,7 @@ impl DomTree { MatchingMode::Normal, None, &mut caches, - QuirksMode::NoQuirks, + self.selector_quirks_mode(), NeedsSelectorFlags::No, MatchingForInvalidation::No, ); @@ -729,6 +744,34 @@ mod tests { assert!(result.is_none()); } + #[test] + fn quirks_mode_matches_class_and_id_case_insensitively() { + // No doctype => quirks mode; class/id match ASCII case-insensitively. + let tree = parse_html(r#"
x
"#); + assert!( + tree.query_selector(".foo").unwrap().is_some(), + ".foo should match class=\"Foo\" in quirks mode" + ); + assert!( + tree.query_selector("#bar").unwrap().is_some(), + "#bar should match id=\"Bar\" in quirks mode" + ); + } + + #[test] + fn standards_mode_matches_class_and_id_case_sensitively() { + // With a doctype => no-quirks; class/id remain case-sensitive. + let tree = parse_html(r#"
x
"#); + assert!( + tree.query_selector(".foo").unwrap().is_none(), + ".foo must NOT match class=\"Foo\" in standards mode" + ); + assert!( + tree.query_selector("#bar").unwrap().is_none(), + "#bar must NOT match id=\"Bar\" in standards mode" + ); + } + #[test] fn test_query_selector_complex() { let tree = parse_html( diff --git a/crates/obscura-dom/src/tree.rs b/crates/obscura-dom/src/tree.rs index 01db8af..9d0f6d8 100644 --- a/crates/obscura-dom/src/tree.rs +++ b/crates/obscura-dom/src/tree.rs @@ -143,6 +143,9 @@ pub(crate) struct DomTreeInner { pub(crate) free_list: Vec, pub(crate) document: NodeId, pub(crate) id_index: HashMap, + // Whether the document was parsed in (full) quirks mode. In quirks mode CSS + // class and id selectors match ASCII-case-insensitively. + pub(crate) quirks: bool, } impl DomTree { @@ -162,6 +165,7 @@ impl DomTree { free_list: Vec::new(), document: NodeId(0), id_index: HashMap::new(), + quirks: false, }), } } @@ -170,6 +174,17 @@ impl DomTree { self.inner.borrow().document } + /// Record whether the document was parsed in (full) quirks mode. + pub fn set_quirks(&self, quirks: bool) { + self.inner.borrow_mut().quirks = quirks; + } + + /// Whether the document is in (full) quirks mode, in which CSS class and id + /// selectors match ASCII-case-insensitively. + pub fn is_quirks(&self) -> bool { + self.inner.borrow().quirks + } + pub(crate) fn borrow_inner(&self) -> std::cell::Ref<'_, DomTreeInner> { self.inner.borrow() } @@ -469,12 +484,20 @@ impl DomTree { inner.id_index.remove(&id_str); } + // Only free slots that are currently live. Freeing an out-of-range id + // would panic on direct indexing, and freeing an already-freed slot + // would push it onto the free list a second time — later handing the + // same NodeId to two live nodes (aliasing). for desc_id in descendants { - inner.nodes[desc_id.index()] = None; - inner.free_list.push(desc_id.0); + if matches!(inner.nodes.get(desc_id.index()), Some(Some(_))) { + inner.nodes[desc_id.index()] = None; + inner.free_list.push(desc_id.0); + } + } + if matches!(inner.nodes.get(node_id.index()), Some(Some(_))) { + inner.nodes[node_id.index()] = None; + inner.free_list.push(node_id.0); } - inner.nodes[node_id.index()] = None; - inner.free_list.push(node_id.0); } pub fn children(&self, node_id: NodeId) -> Vec { @@ -932,6 +955,30 @@ mod tests { assert!(node.is_document()); } + #[test] + fn remove_out_of_range_id_is_a_noop() { + let tree = DomTree::new(); + // Direct indexing into `nodes` panicked out-of-bounds for an id past the + // end of the slot vector; it must be a no-op instead. + tree.remove(NodeId::new(9999)); + assert_eq!(tree.len(), 1); + } + + #[test] + fn remove_twice_does_not_alias_slots() { + let tree = DomTree::new(); + let doc = tree.document(); + let a = tree.new_node(NodeData::Text { contents: "a".into() }); + tree.append_child(doc, a); + tree.remove(a); + // Removing the already-freed node again must not push its slot onto the + // free list a second time, or two later allocations alias one slot. + tree.remove(a); + let x = tree.new_node(NodeData::Text { contents: "x".into() }); + let y = tree.new_node(NodeData::Text { contents: "y".into() }); + assert_ne!(x, y, "double-free aliased two live nodes onto the same slot"); + } + #[test] fn test_append_child() { let tree = DomTree::new(); diff --git a/crates/obscura-dom/src/tree_sink.rs b/crates/obscura-dom/src/tree_sink.rs index 674732f..e116b5a 100644 --- a/crates/obscura-dom/src/tree_sink.rs +++ b/crates/obscura-dom/src/tree_sink.rs @@ -218,7 +218,10 @@ impl TreeSink for DomTree { x == y } - fn set_quirks_mode(&self, _mode: QuirksMode) { + fn set_quirks_mode(&self, mode: QuirksMode) { + // Only full quirks mode makes CSS class/id selectors case-insensitive; + // limited-quirks behaves like no-quirks for selector matching. + self.set_quirks(mode == QuirksMode::Quirks); } fn is_mathml_annotation_xml_integration_point(&self, target: &NodeId) -> bool {