Bug 1363088 - Remove text-shadow handling from HasAuthorSpecifiedRules. r?heycam draft
authorXidorn Quan <me@upsuper.org>
Thu, 12 Oct 2017 11:13:07 +1100
changeset 678951 529ed47dd810819fdc1e7f6c3deb2acdac0aa589
parent 678897 15aaca5a1bcff5387eda0a76e39ac91886cf382b
child 735474 16bccbacae7ba04c649b5d120aa02f3384cfc4f1
push id84079
push userxquan@mozilla.com
push dateThu, 12 Oct 2017 02:59:44 +0000
reviewersheycam
bugs1363088, 1401825
milestone58.0a1
Bug 1363088 - Remove text-shadow handling from HasAuthorSpecifiedRules. r?heycam After bug 1401825, we no longer need the code for text-shadow anymore, so we can just remove it. MozReview-Commit-ID: B2zpzetwW91
layout/base/nsPresContext.h
layout/style/nsRuleNode.cpp
servo/components/style/gecko/generated/structs.rs
servo/components/style/rule_tree/mod.rs
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -114,17 +114,16 @@ enum nsLayoutPhase {
   eLayoutPhase_COUNT
 };
 #endif
 
 /* Used by nsPresContext::HasAuthorSpecifiedRules */
 #define NS_AUTHOR_SPECIFIED_BACKGROUND      (1 << 0)
 #define NS_AUTHOR_SPECIFIED_BORDER          (1 << 1)
 #define NS_AUTHOR_SPECIFIED_PADDING         (1 << 2)
-#define NS_AUTHOR_SPECIFIED_TEXT_SHADOW     (1 << 3)
 
 class nsRootPresContext;
 
 // An interface for presentation contexts. Presentation contexts are
 // objects that provide an outer context for a presentation shell.
 
 class nsPresContext : public nsISupports,
                       public mozilla::SupportsWeakPtr<nsPresContext> {
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -10502,48 +10502,35 @@ nsRuleNode::HasAuthorSpecifiedRules(Geck
     inheritBits |= NS_STYLE_INHERIT_BIT(Background);
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BORDER)
     inheritBits |= NS_STYLE_INHERIT_BIT(Border);
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING)
     inheritBits |= NS_STYLE_INHERIT_BIT(Padding);
 
-  if (ruleTypeMask & NS_AUTHOR_SPECIFIED_TEXT_SHADOW)
-    inheritBits |= NS_STYLE_INHERIT_BIT(Text);
-
   // properties in the SIDS, whether or not we care about them
   size_t nprops = 0,
-         backgroundOffset, borderOffset, paddingOffset, textShadowOffset;
-
-  // We put the reset properties the start of the nsCSSValue array....
+         backgroundOffset, borderOffset, paddingOffset;
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BACKGROUND) {
     backgroundOffset = nprops;
     nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Background);
   }
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BORDER) {
     borderOffset = nprops;
     nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Border);
   }
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) {
     paddingOffset = nprops;
     nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Padding);
   }
 
-  // ...and the inherited properties at the end of the array.
-  size_t inheritedOffset = nprops;
-
-  if (ruleTypeMask & NS_AUTHOR_SPECIFIED_TEXT_SHADOW) {
-    textShadowOffset = nprops;
-    nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Text);
-  }
-
   void* dataStorage = alloca(nprops * sizeof(nsCSSValue));
   AutoCSSValueArray dataArray(dataStorage, nprops);
 
   /* We're relying on the use of |styleContext| not mutating it! */
   nsRuleData ruleData(inheritBits, dataArray.get(),
                       styleContext->PresContext(), styleContext);
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BACKGROUND) {
@@ -10553,20 +10540,16 @@ nsRuleNode::HasAuthorSpecifiedRules(Geck
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BORDER) {
     ruleData.mValueOffsets[eStyleStruct_Border] = borderOffset;
   }
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) {
     ruleData.mValueOffsets[eStyleStruct_Padding] = paddingOffset;
   }
 
-  if (ruleTypeMask & NS_AUTHOR_SPECIFIED_TEXT_SHADOW) {
-    ruleData.mValueOffsets[eStyleStruct_Text] = textShadowOffset;
-  }
-
   static const nsCSSPropertyID backgroundValues[] = {
     eCSSProperty_background_color,
     eCSSProperty_background_image,
   };
 
   static const nsCSSPropertyID borderValues[] = {
     eCSSProperty_border_top_color,
     eCSSProperty_border_top_style,
@@ -10588,32 +10571,26 @@ nsRuleNode::HasAuthorSpecifiedRules(Geck
 
   static const nsCSSPropertyID paddingValues[] = {
     eCSSProperty_padding_top,
     eCSSProperty_padding_right,
     eCSSProperty_padding_bottom,
     eCSSProperty_padding_left,
   };
 
-  static const nsCSSPropertyID textShadowValues[] = {
-    eCSSProperty_text_shadow
-  };
-
   // Number of properties we care about
   size_t nValues = 0;
 
   nsCSSValue* values[MOZ_ARRAY_LENGTH(backgroundValues) +
                      MOZ_ARRAY_LENGTH(borderValues) +
-                     MOZ_ARRAY_LENGTH(paddingValues) +
-                     MOZ_ARRAY_LENGTH(textShadowValues)];
+                     MOZ_ARRAY_LENGTH(paddingValues)];
 
   nsCSSPropertyID properties[MOZ_ARRAY_LENGTH(backgroundValues) +
                            MOZ_ARRAY_LENGTH(borderValues) +
-                           MOZ_ARRAY_LENGTH(paddingValues) +
-                           MOZ_ARRAY_LENGTH(textShadowValues)];
+                           MOZ_ARRAY_LENGTH(paddingValues)];
 
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BACKGROUND) {
     for (uint32_t i = 0, i_end = ArrayLength(backgroundValues);
          i < i_end; ++i) {
       properties[nValues] = backgroundValues[i];
       values[nValues++] = ruleData.ValueFor(backgroundValues[i]);
     }
   }
@@ -10629,24 +10606,16 @@ nsRuleNode::HasAuthorSpecifiedRules(Geck
   if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) {
     for (uint32_t i = 0, i_end = ArrayLength(paddingValues);
          i < i_end; ++i) {
       properties[nValues] = paddingValues[i];
       values[nValues++] = ruleData.ValueFor(paddingValues[i]);
     }
   }
 
-  if (ruleTypeMask & NS_AUTHOR_SPECIFIED_TEXT_SHADOW) {
-    for (uint32_t i = 0, i_end = ArrayLength(textShadowValues);
-         i < i_end; ++i) {
-      properties[nValues] = textShadowValues[i];
-      values[nValues++] = ruleData.ValueFor(textShadowValues[i]);
-    }
-  }
-
   GeckoStyleContext* styleContextRef = styleContext;
 
   // We need to be careful not to count styles covered up by user-important or
   // UA-important declarations.  But we do want to catch explicit inherit
   // styling in those and check our parent style context to see whether we have
   // user styling for those properties.  Note that we don't care here about
   // inheritance due to lack of a specified value, since all the properties we
   // care about are reset properties.
@@ -10667,18 +10636,17 @@ nsRuleNode::HasAuthorSpecifiedRules(Geck
           // This is a rule whose effect we want to ignore, so if any of
           // the properties we care about were set, set them to the dummy
           // value that they'll never otherwise get.
           for (uint32_t i = 0; i < nValues; ++i) {
             nsCSSUnit unit = values[i]->GetUnit();
             if (unit != eCSSUnit_Null &&
                 unit != eCSSUnit_Dummy &&
                 unit != eCSSUnit_DummyInherit) {
-              if (unit == eCSSUnit_Inherit ||
-                  (i >= inheritedOffset && unit == eCSSUnit_Unset)) {
+              if (unit == eCSSUnit_Inherit) {
                 haveExplicitUAInherit = true;
                 values[i]->SetDummyInheritValue();
               } else {
                 values[i]->SetDummyValue();
               }
             }
           }
         } else {
--- a/servo/components/style/gecko/generated/structs.rs
+++ b/servo/components/style/gecko/generated/structs.rs
@@ -954,17 +954,16 @@ pub mod root {
     pub const kNameSpaceID_SVG: ::std::os::raw::c_uint = 10;
     pub const kNameSpaceID_disabled_MathML: ::std::os::raw::c_uint = 11;
     pub const kNameSpaceID_disabled_SVG: ::std::os::raw::c_uint = 12;
     pub const kNameSpaceID_LastBuiltin: ::std::os::raw::c_uint = 12;
     pub const kNameSpaceID_Wildcard: ::std::os::raw::c_int = -2147483648;
     pub const NS_AUTHOR_SPECIFIED_BACKGROUND: ::std::os::raw::c_uint = 1;
     pub const NS_AUTHOR_SPECIFIED_BORDER: ::std::os::raw::c_uint = 2;
     pub const NS_AUTHOR_SPECIFIED_PADDING: ::std::os::raw::c_uint = 4;
-    pub const NS_AUTHOR_SPECIFIED_TEXT_SHADOW: ::std::os::raw::c_uint = 8;
     pub const NS_STYLE_INHERIT_MASK: ::std::os::raw::c_uint = 16777215;
     pub const NS_STYLE_HAS_TEXT_DECORATION_LINES: ::std::os::raw::c_uint =
         16777216;
     pub const NS_STYLE_HAS_PSEUDO_ELEMENT_DATA: ::std::os::raw::c_uint =
         33554432;
     pub const NS_STYLE_RELEVANT_LINK_VISITED: ::std::os::raw::c_uint =
         67108864;
     pub const NS_STYLE_IS_STYLE_IF_VISITED: ::std::os::raw::c_uint =
--- a/servo/components/style/rule_tree/mod.rs
+++ b/servo/components/style/rule_tree/mod.rs
@@ -1081,18 +1081,19 @@ impl StrongRuleNode {
                                          mut element: E,
                                          mut pseudo: Option<PseudoElement>,
                                          guards: &StylesheetGuards,
                                          rule_type_mask: u32,
                                          author_colors_allowed: bool)
         -> bool
         where E: ::dom::TElement
     {
-        use gecko_bindings::structs::{NS_AUTHOR_SPECIFIED_BACKGROUND, NS_AUTHOR_SPECIFIED_BORDER};
-        use gecko_bindings::structs::{NS_AUTHOR_SPECIFIED_PADDING, NS_AUTHOR_SPECIFIED_TEXT_SHADOW};
+        use gecko_bindings::structs::NS_AUTHOR_SPECIFIED_BACKGROUND;
+        use gecko_bindings::structs::NS_AUTHOR_SPECIFIED_BORDER;
+        use gecko_bindings::structs::NS_AUTHOR_SPECIFIED_PADDING;
         use properties::{CSSWideKeyword, LonghandId, LonghandIdSet};
         use properties::{PropertyDeclaration, PropertyDeclarationId};
         use std::borrow::Cow;
         use values::specified::Color;
 
         // Reset properties:
         const BACKGROUND_PROPS: &'static [LonghandId] = &[
             LonghandId::BackgroundColor,
@@ -1138,25 +1139,16 @@ impl StrongRuleNode {
             LonghandId::PaddingLeft,
 
             LonghandId::PaddingInlineStart,
             LonghandId::PaddingInlineEnd,
             LonghandId::PaddingBlockStart,
             LonghandId::PaddingBlockEnd,
         ];
 
-        // Inherited properties:
-        const TEXT_SHADOW_PROPS: &'static [LonghandId] = &[
-            LonghandId::TextShadow,
-        ];
-
-        fn inherited(id: LonghandId) -> bool {
-            id == LonghandId::TextShadow
-        }
-
         // Set of properties that we are currently interested in.
         let mut properties = LonghandIdSet::new();
 
         if rule_type_mask & NS_AUTHOR_SPECIFIED_BACKGROUND != 0 {
             for id in BACKGROUND_PROPS {
                 properties.insert(*id);
             }
         }
@@ -1165,36 +1157,30 @@ impl StrongRuleNode {
                 properties.insert(*id);
             }
         }
         if rule_type_mask & NS_AUTHOR_SPECIFIED_PADDING != 0 {
             for id in PADDING_PROPS {
                 properties.insert(*id);
             }
         }
-        if rule_type_mask & NS_AUTHOR_SPECIFIED_TEXT_SHADOW != 0 {
-            for id in TEXT_SHADOW_PROPS {
-                properties.insert(*id);
-            }
-        }
 
         // If author colors are not allowed, only claim to have author-specified
         // rules if we're looking at a non-color property or if we're looking at
         // the background color and it's set to transparent.
         const IGNORED_WHEN_COLORS_DISABLED: &'static [LonghandId]  = &[
             LonghandId::BackgroundImage,
             LonghandId::BorderTopColor,
             LonghandId::BorderRightColor,
             LonghandId::BorderBottomColor,
             LonghandId::BorderLeftColor,
             LonghandId::BorderInlineStartColor,
             LonghandId::BorderInlineEndColor,
             LonghandId::BorderBlockStartColor,
             LonghandId::BorderBlockEndColor,
-            LonghandId::TextShadow,
         ];
 
         if !author_colors_allowed {
             for id in IGNORED_WHEN_COLORS_DISABLED {
                 properties.remove(*id);
             }
         }
 
@@ -1203,24 +1189,16 @@ impl StrongRuleNode {
         loop {
             // We need to be careful not to count styles covered up by
             // user-important or UA-important declarations.  But we do want to
             // catch explicit inherit styling in those and check our parent
             // element to see whether we have user styling for those properties.
             // Note that we don't care here about inheritance due to lack of a
             // specified value, since all the properties we care about are reset
             // properties.
-            //
-            // FIXME: The above comment is copied from Gecko, but the last
-            // sentence is no longer correct since 'text-shadow' support was
-            // added.
-            //
-            // This is a bug in Gecko, replicated in Stylo for now:
-            //
-            // https://bugzilla.mozilla.org/show_bug.cgi?id=1363088
 
             let mut inherited_properties = LonghandIdSet::new();
             let mut have_explicit_ua_inherit = false;
 
             for node in element_rule_node.self_and_ancestors() {
                 let source = node.style_source();
                 let declarations = if source.is_some() {
                     source.read(node.cascade_level().guard(guards)).declaration_importance_iter()
@@ -1252,20 +1230,17 @@ impl StrongRuleNode {
                                 // This property was set by a non-author rule.
                                 // Stop looking for it in this element's rule
                                 // nodes.
                                 properties.remove(id);
 
                                 // However, if it is inherited, then it might be
                                 // inherited from an author rule from an
                                 // ancestor element's rule nodes.
-                                if declaration.get_css_wide_keyword() == Some(CSSWideKeyword::Inherit) ||
-                                    (declaration.get_css_wide_keyword() == Some(CSSWideKeyword::Unset) &&
-                                     inherited(id))
-                                {
+                                if declaration.get_css_wide_keyword() == Some(CSSWideKeyword::Inherit) {
                                     have_explicit_ua_inherit = true;
                                     inherited_properties.insert(id);
                                 }
                             }
                         }
                     }
                     // Author rules:
                     CascadeLevel::PresHints |