Bug 1513920 - Make <use> shadow trees lookup keyframe rules in the containing tree. r=heycam, a=RyanVM
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 17 Dec 2018 13:14:19 +0000
changeset 509101 b3837d6171d2c848d82fc7de31c0e84d9eacff14
parent 509100 dd09f113cb8a6936dd8d92b7b5a425eb43019af0
child 509102 fde04f5ec92099bf5cd7d1328f0f09e3d5ff610a
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, RyanVM
bugs1513920
milestone65.0
Bug 1513920 - Make <use> shadow trees lookup keyframe rules in the containing tree. r=heycam, a=RyanVM The same thing we do for rule matching. Differential Revision: https://phabricator.services.mozilla.com/D14548
servo/components/style/rule_collector.rs
servo/components/style/stylist.rs
testing/web-platform/tests/svg/linking/reftests/use-keyframes-ref.html
testing/web-platform/tests/svg/linking/reftests/use-keyframes.html
--- a/servo/components/style/rule_collector.rs
+++ b/servo/components/style/rule_collector.rs
@@ -1,27 +1,64 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/. */
 
 //! Collects a series of applicable rules for a given element.
 
 use crate::applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList};
-use crate::dom::{TElement, TShadowRoot};
+use crate::dom::{TElement, TNode, TShadowRoot};
 use crate::properties::{AnimationRules, PropertyDeclarationBlock};
 use crate::rule_tree::{CascadeLevel, ShadowCascadeOrder};
 use crate::selector_map::SelectorMap;
 use crate::selector_parser::PseudoElement;
 use crate::shared_lock::Locked;
 use crate::stylesheets::Origin;
 use crate::stylist::{AuthorStylesEnabled, Rule, RuleInclusion, Stylist};
 use selectors::matching::{ElementSelectorFlags, MatchingContext};
 use servo_arc::ArcBorrow;
 use smallvec::SmallVec;
 
+/// This is a bit of a hack so <svg:use> matches the rules of the enclosing
+/// tree.
+///
+/// This function returns the containing shadow host ignoring <svg:use> shadow
+/// trees, since those match the enclosing tree's rules.
+///
+/// Only a handful of places need to really care about this. 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
+/// <svg:use> subtrees are immutable and recreated each time the source tree
+/// changes.
+///
+/// We historically allow cross-document <svg:use> to have these rules applied,
+/// but I think that's not great. Gecko is the only engine supporting that.
+///
+/// See https://github.com/w3c/svgwg/issues/504 for the relevant spec
+/// discussion.
+#[inline]
+pub fn containing_shadow_ignoring_svg_use<E: TElement>(
+    element: E,
+) -> Option<<E::ConcreteNode as TNode>::ConcreteShadowRoot> {
+    let mut shadow = element.containing_shadow()?;
+    loop {
+        let host = shadow.host();
+        let host_is_svg_use_element =
+            host.is_svg_element() && host.local_name() == &*local_name!("use");
+        if !host_is_svg_use_element {
+            return Some(shadow);
+        }
+        debug_assert!(
+            shadow.style_data().is_none(),
+            "We allow no stylesheets in <svg:use> subtrees"
+        );
+        shadow = host.containing_shadow()?;
+    }
+}
+
 /// An object that we use with all the intermediate state needed for the
 /// cascade.
 ///
 /// This is done basically to be able to organize the cascade in smaller
 /// functions, and be able to reason about it easily.
 pub struct RuleCollector<'a, 'b: 'a, E, F: 'a>
 where
     E: TElement,
@@ -208,53 +245,29 @@ where
         }
     }
 
     fn collect_normal_rules_from_containing_shadow_tree(&mut self) {
         if !self.matches_user_and_author_rules {
             return;
         }
 
-        let mut current_containing_shadow = self.rule_hash_target.containing_shadow();
-        while let Some(containing_shadow) = current_containing_shadow {
-            let cascade_data = containing_shadow.style_data();
-            let host = containing_shadow.host();
-            if let Some(map) = cascade_data.and_then(|data| data.normal_rules(self.pseudo_element))
-            {
-                self.collect_rules_in_shadow_tree(host, map, CascadeLevel::SameTreeAuthorNormal);
-            }
-            let host_is_svg_use_element =
-                host.is_svg_element() && host.local_name() == &*local_name!("use");
-            if !host_is_svg_use_element {
-                self.matches_document_author_rules = false;
-                break;
-            }
+        let containing_shadow = containing_shadow_ignoring_svg_use(self.rule_hash_target);
+        let containing_shadow = match containing_shadow {
+            Some(s) => s,
+            None => return,
+        };
 
-            debug_assert!(
-                cascade_data.is_none(),
-                "We allow no stylesheets in <svg:use> subtrees"
-            );
+        self.matches_document_author_rules = false;
 
-            // NOTE(emilio): Hack so <svg:use> matches the rules of the
-            // enclosing tree.
-            //
-            // 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 <svg:use>
-            // subtrees are immutable and recreated each time the source
-            // tree changes.
-            //
-            // We historically allow cross-document <svg:use> to have these
-            // rules applied, but I think that's not great. Gecko is the
-            // only engine supporting that.
-            //
-            // See https://github.com/w3c/svgwg/issues/504 for the relevant
-            // spec discussion.
-            current_containing_shadow = host.containing_shadow();
-            self.matches_document_author_rules = current_containing_shadow.is_none();
+        let cascade_data = containing_shadow.style_data();
+        let host = containing_shadow.host();
+        if let Some(map) = cascade_data.and_then(|data| data.normal_rules(self.pseudo_element))
+        {
+            self.collect_rules_in_shadow_tree(host, map, CascadeLevel::SameTreeAuthorNormal);
         }
     }
 
     /// Collects the rules for the :host pseudo-class.
     fn collect_host_rules(&mut self) {
         let shadow = match self.rule_hash_target.shadow_root() {
             Some(s) => s,
             None => return,
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -12,17 +12,17 @@ use crate::font_metrics::FontMetricsProv
 #[cfg(feature = "gecko")]
 use crate::gecko_bindings::structs::{ServoStyleSetSizes, StyleRuleInclusion};
 use crate::invalidation::element::invalidation_map::InvalidationMap;
 use crate::invalidation::media_queries::{EffectiveMediaQueryResults, ToMediaListKey};
 use crate::media_queries::Device;
 use crate::properties::{self, CascadeMode, ComputedValues};
 use crate::properties::{AnimationRules, PropertyDeclarationBlock};
 use crate::rule_cache::{RuleCache, RuleCacheConditions};
-use crate::rule_collector::RuleCollector;
+use crate::rule_collector::{RuleCollector, containing_shadow_ignoring_svg_use};
 use crate::rule_tree::{CascadeLevel, RuleTree, ShadowCascadeOrder, StrongRuleNode, StyleSource};
 use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, SelectorMap, SelectorMapEntry};
 use crate::selector_parser::{PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap};
 use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use crate::stylesheet_set::{DataValidity, DocumentStylesheetSet, SheetRebuildKind};
 use crate::stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher};
 use crate::stylesheets::keyframes_rule::KeyframesAnimation;
 use crate::stylesheets::viewport_rule::{self, MaybeNew, ViewportRule};
@@ -1185,17 +1185,19 @@ impl Stylist {
         // [2]: https://github.com/w3c/csswg-drafts/issues/1995
         // [3]: https://bugzil.la/1458189
         if let Some(shadow) = element.shadow_root() {
             if let Some(data) = shadow.style_data() {
                 try_find_in!(data);
             }
         }
 
-        if let Some(shadow) = element.containing_shadow() {
+        // Use the same rules to look for the containing host as we do for rule
+        // collection.
+        if let Some(shadow) = containing_shadow_ignoring_svg_use(element) {
             if let Some(data) = shadow.style_data() {
                 try_find_in!(data);
             }
         } else {
             try_find_in!(self.cascade_data.author);
         }
 
         try_find_in!(self.cascade_data.user);
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/svg/linking/reftests/use-keyframes-ref.html
@@ -0,0 +1,6 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<svg width="100" height="100" viewBox="0 0 100 100">
+  <rect width="100%" height="100%" fill="lime" />
+</svg>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/svg/linking/reftests/use-keyframes.html
@@ -0,0 +1,19 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: Keyframe animations from the document match in use elements</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1513920">
+<link rel="match" href="use-keyframes-ref.html">
+<style>
+@keyframes animationname {
+  from { fill: lime; }
+  to { fill: lime; }
+}
+</style>
+<svg width="100" height="100" viewBox="0 0 100 100">
+  <symbol id="symbol">
+    <rect width="100%" height="100%" fill="red" style="animation: animationname 1s infinite;" />
+  </symbol>
+  <use href="#symbol" />
+</svg>