From 02d27ad3dd771e7138c0b856d0a2cea248b9835f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 16 Jul 2018 16:07:58 +0200 Subject: [PATCH] style: Make svg:use use an actual shadow tree. This fixes a couple fuzz bugs and prevents special-casing even more in bug 1431255. Unfortunately not as many hacks went away as I'd have hoped, since we still need to match document rules, see the linked SVGWG issues. But blocks_ancestor_combinators goes away, which is nice since it's on a very hot path. Bug: 1450250 Reviewed-by: heycam Differential Revision: https://phabricator.services.mozilla.com/D2154 MozReview-Commit-ID: C4mthjoSNFh --- components/selectors/matching.rs | 4 ---- components/selectors/tree.rs | 7 ------ components/style/gecko/wrapper.rs | 15 +++--------- .../invalidation/element/element_wrapper.rs | 4 ---- components/style/stylist.rs | 24 +++++++++++++++++-- 5 files changed, 25 insertions(+), 29 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 7c4812cb963..fd4765c2dff 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -418,10 +418,6 @@ where match combinator { Combinator::NextSibling | Combinator::LaterSibling => element.prev_sibling_element(), Combinator::Child | Combinator::Descendant => { - if element.blocks_ancestor_combinators() { - return None; - } - match element.parent_element() { Some(e) => return Some(e), None => {}, diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index b1c0d41caa8..ef544cc7e4c 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -129,11 +129,4 @@ pub trait Element: Sized + Clone + Debug { fn ignores_nth_child_selectors(&self) -> bool { false } - - /// Return true if we want to stop lookup ancestor of the current - /// element while matching complex selectors with descendant/child - /// combinator. - fn blocks_ancestor_combinators(&self) -> bool { - false - } } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 035cf0f7821..6cdcf1acd1e 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -856,13 +856,12 @@ impl<'le> GeckoElement<'le> { /// Returns true if this node is the shadow root of an use-element shadow tree. #[inline] fn is_root_of_use_element_shadow_tree(&self) -> bool { - if !self.is_root_of_anonymous_subtree() { + if !self.as_node().is_in_shadow_tree() { return false; } - match self.parent_element() { + match self.containing_shadow_host() { Some(e) => { - e.local_name() == &*local_name!("use") && - e.is_svg_element() + e.is_svg_element() && e.local_name() == &*local_name!("use") }, None => false, } @@ -2332,14 +2331,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { fn ignores_nth_child_selectors(&self) -> bool { self.is_root_of_anonymous_subtree() } - - #[inline] - fn blocks_ancestor_combinators(&self) -> bool { - // If this element is the shadow root of an use-element shadow tree, - // according to the spec, we should not match rules cross the shadow - // DOM boundary. - self.is_root_of_use_element_shadow_tree() - } } /// A few helpers to help with attribute selectors and snapshotting. diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index 2a31153bd7a..ecbc3b94b78 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/components/style/invalidation/element/element_wrapper.rs @@ -367,8 +367,4 @@ where .assigned_slot() .map(|e| ElementWrapper::new(e, self.snapshot_map)) } - - fn blocks_ancestor_combinators(&self) -> bool { - self.element.blocks_ancestor_combinators() - } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 820f4dbcda7..f4614f945af 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1261,8 +1261,9 @@ impl Stylist { if let Some(containing_shadow) = rule_hash_target.containing_shadow() { let cascade_data = containing_shadow.style_data(); + let host = containing_shadow.host(); if let Some(map) = cascade_data.normal_rules(pseudo_element) { - context.with_shadow_host(Some(containing_shadow.host()), |context| { + context.with_shadow_host(Some(host), |context| { map.get_all_matching_rules( element, rule_hash_target, @@ -1276,7 +1277,26 @@ impl Stylist { shadow_cascade_order += 1; } - match_document_author_rules = false; + // NOTE(emilio): Hack so matches document rules as + // expected. + // + // This is not a problem for invalidation and that kind of stuff + // because they still don't match rules based on elements + // outside of the shadow tree, and because the subtree + // is immutable and recreated each time the source tree changes. + // + // See: https://github.com/w3c/svgwg/issues/504 + // + // Note that we always resolve URLs against the document, so we + // can't get into a nested shadow situation here. + // + // See: https://github.com/w3c/svgwg/issues/505 + // + let host_is_svg_use = + host.is_svg_element() && + host.local_name() == &*local_name!("use"); + + match_document_author_rules = host_is_svg_use; } }