Remove initial value of @counter-style rule descriptors. r?emilio draft
authorXidorn Quan <me@upsuper.org>
Wed, 04 Apr 2018 13:48:02 +1000
changeset 777169 c3a8a5987485d1f8c671dbb6409597557c97f42d
parent 777168 35bc7b09c919bc0333565c33ed19e1f2b87b8122
child 777170 7c5d79df82069e532c40d9c7f8810064b0214062
push id105089
push userxquan@mozilla.com
push dateWed, 04 Apr 2018 10:15:09 +0000
reviewersemilio
milestone61.0a1
Remove initial value of @counter-style rule descriptors. r?emilio This is misleading and makes its accessors harder to use. For counter styles, a descriptor being unspecified doesn't mean it would take the initial value. If the system is `extends`, they need to take corresponding value from the counter style it's extending. Since details of the extended counter style is unknown for the style itself, it is not possible to report value for unspecified descriptor in that case. The "system" descriptor itself, though, should always have a resolved value (otherwise we would know nothing about the counter style). When it's not specified, it defaults to `symbolic`. To make things convenient a separate method `get_system` is added for that. MozReview-Commit-ID: Cp36NdHZ3sU
servo/components/style/counter_style/mod.rs
--- a/servo/components/style/counter_style/mod.rs
+++ b/servo/components/style/counter_style/mod.rs
@@ -10,17 +10,16 @@ use Atom;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser};
 use cssparser::{Parser, Token, CowRcStr};
 use error_reporting::{ContextualParseError, ParseErrorReporter};
 #[cfg(feature = "gecko")] use gecko::rules::CounterStyleDescriptors;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::{ nsCSSCounterDesc, nsCSSValue };
 use parser::{ParserContext, ParserErrorContext, Parse};
 use selectors::parser::SelectorParseErrorKind;
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
-use std::borrow::Cow;
 use std::fmt::{self, Write};
 use std::ops::Range;
 use str::CssStringWriter;
 use style_traits::{Comma, CssWriter, OneOrMoreSeparated, ParseError};
 use style_traits::{StyleParseErrorKind, ToCss};
 use values::CustomIdent;
 use values::specified::Integer;
 
@@ -89,17 +88,17 @@ pub fn parse_counter_style_body<'i, 't, 
         while let Some(declaration) = iter.next() {
             if let Err((error, slice)) = declaration {
                 let location = error.location;
                 let error = ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(slice, error);
                 context.log_css_error(error_context, location, error)
             }
         }
     }
-    let error = match *rule.system() {
+    let error = match *rule.get_system() {
         ref system @ System::Cyclic |
         ref system @ System::Fixed { .. } |
         ref system @ System::Symbolic |
         ref system @ System::Alphabetic |
         ref system @ System::Numeric
         if rule.symbols.is_none() => {
             let system = system.to_css_string();
             Some(ContextualParseError::InvalidCounterStyleWithoutSymbols(system))
@@ -137,39 +136,19 @@ struct CounterStyleRuleParser<'a, 'b: 'a
 /// Default methods reject all at rules.
 impl<'a, 'b, 'i> AtRuleParser<'i> for CounterStyleRuleParser<'a, 'b> {
     type PreludeNoBlock = ();
     type PreludeBlock = ();
     type AtRule = ();
     type Error = StyleParseErrorKind<'i>;
 }
 
-macro_rules! accessor {
-    (#[$doc: meta] $name: tt $ident: ident: $ty: ty = !) => {
-        #[$doc]
-        pub fn $ident(&self) -> Option<&$ty> {
-            self.$ident.as_ref()
-        }
-    };
-
-    (#[$doc: meta] $name: tt $ident: ident: $ty: ty = $initial: expr) => {
-        #[$doc]
-        pub fn $ident(&self) -> Cow<$ty> {
-            if let Some(ref value) = self.$ident {
-                Cow::Borrowed(value)
-            } else {
-                Cow::Owned($initial)
-            }
-        }
-    }
-}
-
 macro_rules! counter_style_descriptors {
     (
-        $( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty = $initial: tt )+
+        $( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty, )+
     ) => {
         /// An @counter-style rule
         #[derive(Clone, Debug)]
         pub struct CounterStyleRuleData {
             name: CustomIdent,
             $(
                 #[$doc]
                 $ident: Option<$ty>,
@@ -186,18 +165,30 @@ macro_rules! counter_style_descriptors {
                 }
             }
 
             /// Get the name of the counter style rule.
             pub fn name(&self) -> &CustomIdent {
                 &self.name
             }
 
+            /// Get the system of this counter style rule, default to
+            /// `symbolic` if not specified.
+            pub fn get_system(&self) -> &System {
+                match self.system {
+                    Some(ref system) => system,
+                    None => &System::Symbolic,
+                }
+            }
+
             $(
-                accessor!(#[$doc] $name $ident: $ty = $initial);
+                #[$doc]
+                pub fn $ident(&self) -> Option<&$ty> {
+                    self.$ident.as_ref()
+                }
             )+
 
             /// Convert to Gecko types
             #[cfg(feature = "gecko")]
             pub fn set_descriptors(self, descriptors: &mut CounterStyleDescriptors) {
                 $(
                     if let Some(value) = self.$ident {
                         descriptors[nsCSSCounterDesc::$gecko_ident as usize].set_from(value)
@@ -268,61 +259,44 @@ macro_rules! counter_style_descriptors {
             }
             Ok(())
         }
     }
 }
 
 counter_style_descriptors! {
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-system>
-    "system" system / eCSSCounterDesc_System: System = {
-        System::Symbolic
-    }
+    "system" system / eCSSCounterDesc_System: System,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-negative>
-    "negative" negative / eCSSCounterDesc_Negative: Negative = {
-        Negative(Symbol::String("-".to_owned()), None)
-    }
+    "negative" negative / eCSSCounterDesc_Negative: Negative,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-prefix>
-    "prefix" prefix / eCSSCounterDesc_Prefix: Symbol = {
-        Symbol::String("".to_owned())
-    }
+    "prefix" prefix / eCSSCounterDesc_Prefix: Symbol,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-suffix>
-    "suffix" suffix / eCSSCounterDesc_Suffix: Symbol = {
-        Symbol::String(". ".to_owned())
-    }
+    "suffix" suffix / eCSSCounterDesc_Suffix: Symbol,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-range>
-    "range" range / eCSSCounterDesc_Range: Ranges = {
-        Ranges(Vec::new())  // Empty Vec represents 'auto'
-    }
+    "range" range / eCSSCounterDesc_Range: Ranges,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-pad>
-    "pad" pad / eCSSCounterDesc_Pad: Pad = {
-        Pad(Integer::new(0), Symbol::String("".to_owned()))
-    }
+    "pad" pad / eCSSCounterDesc_Pad: Pad,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-fallback>
-    "fallback" fallback / eCSSCounterDesc_Fallback: Fallback = {
-        // FIXME https://bugzilla.mozilla.org/show_bug.cgi?id=1359323 use atom!()
-        Fallback(CustomIdent(Atom::from("decimal")))
-    }
+    "fallback" fallback / eCSSCounterDesc_Fallback: Fallback,
 
     /// <https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-symbols>
-    "symbols" symbols / eCSSCounterDesc_Symbols: Symbols = !
+    "symbols" symbols / eCSSCounterDesc_Symbols: Symbols,
 
     /// <https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-additive-symbols>
-    "additive-symbols" additive_symbols / eCSSCounterDesc_AdditiveSymbols: AdditiveSymbols = !
+    "additive-symbols" additive_symbols / eCSSCounterDesc_AdditiveSymbols: AdditiveSymbols,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-speak-as>
-    "speak-as" speak_as / eCSSCounterDesc_SpeakAs: SpeakAs = {
-        SpeakAs::Auto
-    }
+    "speak-as" speak_as / eCSSCounterDesc_SpeakAs: SpeakAs,
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#counter-style-system>
 #[derive(Clone, Debug)]
 pub enum System {
     /// 'cyclic'
     Cyclic,
     /// 'numeric'