From 4993a80074f1fe3d69c732e65a67480349c48d25 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 05:24:04 +0200 Subject: [PATCH 01/26] Introduce a style::values::CustomIdent type --- .../style/properties/properties.mako.rs | 1 + components/style/values/mod.rs | 32 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 0b34c5193e2..a0234c52bad 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -444,6 +444,7 @@ impl CSSWideKeyword { /// to a CSSWideKeyword. pub fn from_ident<'i>(ident: &Cow<'i, str>) -> Option { match_ignore_ascii_case! { ident, + // If modifying this set of keyword, also update values::CustomIdent::from_ident "initial" => Some(CSSWideKeyword::Initial), "inherit" => Some(CSSWideKeyword::Inherit), "unset" => Some(CSSWideKeyword::Unset), diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index 2b371b74da9..7379cbad846 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -8,8 +8,11 @@ #![deny(missing_docs)] -pub use cssparser::{RGBA, Parser}; +use Atom; +pub use cssparser::{RGBA, Parser, serialize_identifier}; use parser::{Parse, ParserContext}; +use std::ascii::AsciiExt; +use std::borrow::Cow; use std::fmt::{self, Debug}; use style_traits::ToCss; @@ -211,6 +214,33 @@ impl ToComputedValue for Either { } } +/// https://drafts.csswg.org/css-values-4/#custom-idents +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub struct CustomIdent(pub Atom); + +impl CustomIdent { + /// Parse an already-tokenizer identifier + pub fn from_ident(ident: Cow, excluding: &[&str]) -> Result { + match_ignore_ascii_case! { &ident, + "initial" | "inherit" | "unset" | "default" => return Err(()), + _ => {} + }; + if excluding.iter().any(|s| ident.eq_ignore_ascii_case(s)) { + Err(()) + } else { + Ok(CustomIdent(ident.into())) + } + } +} + +impl ToCss for CustomIdent { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + serialize_identifier(&self.0.to_string(), dest) + } +} + + // A type for possible values for min- and max- flavors of width, // height, block-size, and inline-size. define_css_keyword_enum!(ExtremumLength: From d9c2d1a9fb5889cea52731de0695e222f19fe54e Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 05:26:51 +0200 Subject: [PATCH 02/26] Use CustomIdent for animation-name and @keyframes --- components/script/dom/csskeyframesrule.rs | 14 +++------- components/style/animation.rs | 8 +++--- components/style/matching.rs | 2 +- components/style/properties/gecko.mako.rs | 7 +++-- .../style/properties/longhand/box.mako.rs | 28 ++++++++----------- .../style/properties/properties.mako.rs | 2 +- components/style/stylesheets.rs | 13 +++++---- components/style/stylist.rs | 4 +-- 8 files changed, 35 insertions(+), 43 deletions(-) diff --git a/components/script/dom/csskeyframesrule.rs b/components/script/dom/csskeyframesrule.rs index 281ccd972a7..5a03dc41658 100644 --- a/components/script/dom/csskeyframesrule.rs +++ b/components/script/dom/csskeyframesrule.rs @@ -16,11 +16,11 @@ use dom::cssrulelist::{CSSRuleList, RulesSource}; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use servo_atoms::Atom; use std::sync::Arc; use style::keyframes::{Keyframe, KeyframeSelector}; use style::shared_lock::{Locked, ToCssWithGuard}; use style::stylesheets::KeyframesRule; +use style::values::CustomIdent; #[dom_struct] pub struct CSSKeyframesRule { @@ -107,7 +107,7 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule { // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name fn Name(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - DOMString::from(&*self.keyframesrule.read_with(&guard).name) + DOMString::from(&*self.keyframesrule.read_with(&guard).name.0) } // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name @@ -115,15 +115,9 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule { // https://github.com/w3c/csswg-drafts/issues/801 // Setting this property to a CSS-wide keyword or `none` will // throw a Syntax Error. - match_ignore_ascii_case! { &value, - "initial" => return Err(Error::Syntax), - "inherit" => return Err(Error::Syntax), - "unset" => return Err(Error::Syntax), - "none" => return Err(Error::Syntax), - _ => () - } + let name = CustomIdent::from_ident(value.into(), &["none"]).map_err(|()| Error::Syntax)?; let mut guard = self.cssrule.shared_lock().write(); - self.keyframesrule.write_with(&mut guard).name = Atom::from(value); + self.keyframesrule.write_with(&mut guard).name = name; Ok(()) } } diff --git a/components/style/animation.rs b/components/style/animation.rs index 8db5ea2235c..604f9ee62bf 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -470,8 +470,8 @@ pub fn maybe_start_animations(context: &SharedStyleContext, continue } - if let Some(ref anim) = context.stylist.animations().get(&name.0) { - debug!("maybe_start_animations: animation {} found", name); + if let Some(ref anim) = context.stylist.animations().get(&name.0 .0) { + debug!("maybe_start_animations: animation {} found", name.0 .0); // If this animation doesn't have any keyframe, we can just continue // without submitting it to the compositor, since both the first and @@ -506,7 +506,7 @@ pub fn maybe_start_animations(context: &SharedStyleContext, new_animations_sender - .send(Animation::Keyframes(node, name.0.clone(), KeyframesAnimationState { + .send(Animation::Keyframes(node, name.0 .0.clone(), KeyframesAnimationState { started_at: animation_start, duration: duration as f64, delay: delay as f64, @@ -586,7 +586,7 @@ pub fn update_style_for_animation(context: &SharedStyleContext, let maybe_index = style.get_box() .animation_name_iter() - .position(|animation_name| *name == animation_name.0); + .position(|animation_name| *name == animation_name.0 .0); let index = match maybe_index { Some(index) => index, diff --git a/components/style/matching.rs b/components/style/matching.rs index e1c1ba47223..cbaa6d12afb 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -560,7 +560,7 @@ trait PrivateMatchMethods: TElement { pseudo: Option<&PseudoElement>) -> bool { let ref new_box_style = new_values.get_box(); let has_new_animation_style = new_box_style.animation_name_count() >= 1 && - new_box_style.animation_name_at(0).0.len() != 0; + new_box_style.animation_name_at(0).0 .0.len() != 0; let has_animations = self.has_css_animations(pseudo); old_values.as_ref().map_or(has_new_animation_style, |ref old| { diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 5190d6c3baf..ee419a9bf77 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -62,7 +62,7 @@ use std::ptr; use std::sync::Arc; use std::cmp; use values::computed::ToComputedValue; -use values::{Either, Auto}; +use values::{Either, Auto, CustomIdent}; use computed_values::border_style; pub mod style_structs { @@ -2205,7 +2205,7 @@ fn static_assert() { self.gecko.mAnimationNameCount = v.0.len() as u32; for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) { // TODO This is inefficient. We should fix this in bug 1329169. - gecko.mName.assign_utf8(&nsCString::from(servo.0.to_string())); + gecko.mName.assign_utf8(&nsCString::from(servo.0 .0.to_string())); } } pub fn animation_name_at(&self, index: usize) @@ -2213,7 +2213,8 @@ fn static_assert() { use Atom; use properties::longhands::animation_name::single_value::SpecifiedValue as AnimationName; // XXX: Is there any effective ways? - AnimationName(Atom::from(String::from_utf16_lossy(&self.gecko.mAnimations[index].mName[..]))) + AnimationName(CustomIdent(Atom::from( + String::from_utf16_lossy(&self.gecko.mAnimations[index].mName[..])))) } pub fn copy_animation_name_from(&mut self, other: &Self) { unsafe { self.gecko.mAnimations.ensure_len(other.gecko.mAnimations.len()) }; diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 6af83510664..e5aae336283 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -796,7 +796,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", use std::ops::Deref; use style_traits::ToCss; use values::computed::ComputedValueAsSpecified; - use values::HasViewportPercentage; + use values::{HasViewportPercentage, CustomIdent}; pub mod computed_value { pub use super::SpecifiedValue as T; @@ -804,7 +804,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", #[derive(Clone, Debug, Hash, Eq, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct SpecifiedValue(pub Atom); + pub struct SpecifiedValue(pub CustomIdent); #[inline] pub fn get_initial_value() -> computed_value::T { @@ -813,21 +813,21 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", #[inline] pub fn get_initial_specified_value() -> SpecifiedValue { - SpecifiedValue(atom!("")) + SpecifiedValue(CustomIdent(atom!(""))) } impl fmt::Display for SpecifiedValue { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.0.fmt(f) + self.0 .0.fmt(f) } } impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if self.0 == atom!("") { + if self.0 .0 == atom!("") { dest.write_str("none") } else { - dest.write_str(&*self.0.to_string()) + self.0.to_css(dest) } } } @@ -835,23 +835,19 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", impl Parse for SpecifiedValue { fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result { use cssparser::Token; - use properties::CSSWideKeyword; use std::ascii::AsciiExt; + use values::CustomIdent; let atom = match input.next() { - Ok(Token::Ident(ref value)) => { - if CSSWideKeyword::from_ident(value).is_some() { - // We allow any ident for the animation-name except one - // of the CSS-wide keywords. - return Err(()); - } else if value.eq_ignore_ascii_case("none") { + Ok(Token::Ident(value)) => { + if value.eq_ignore_ascii_case("none") { // FIXME We may want to support `@keyframes ""` at some point. - atom!("") + CustomIdent(atom!("")) } else { - Atom::from(&**value) + CustomIdent::from_ident(value, &[])? } } - Ok(Token::QuotedString(value)) => Atom::from(&*value), + Ok(Token::QuotedString(value)) => CustomIdent(Atom::from(value)), _ => return Err(()), }; Ok(SpecifiedValue(atom)) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index a0234c52bad..403399acfa9 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1617,7 +1617,7 @@ pub mod style_structs { /// Returns whether there is any animation specified with /// animation-name other than `none`. pub fn specifies_animations(&self) -> bool { - self.animation_name_iter().any(|name| name.0 != atom!("")) + self.animation_name_iter().any(|name| name.0 .0 != atom!("")) } /// Returns whether there are any transitions specified. diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index b5ac86188d6..90f551cfdf8 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -40,6 +40,7 @@ use str::starts_with_ignore_ascii_case; use style_traits::ToCss; use stylist::FnvHashMap; use supports::SupportsCondition; +use values::CustomIdent; use values::specified::url::SpecifiedUrl; use viewport::ViewportRule; @@ -513,7 +514,7 @@ impl ToCssWithGuard for ImportRule { #[derive(Debug)] pub struct KeyframesRule { /// The name of the current animation. - pub name: Atom, + pub name: CustomIdent, /// The keyframes specified for this CSS rule. pub keyframes: Vec>>, /// Vendor prefix type the @keyframes has. @@ -525,7 +526,7 @@ impl ToCssWithGuard for KeyframesRule { fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result where W: fmt::Write { try!(dest.write_str("@keyframes ")); - try!(dest.write_str(&*self.name.to_string())); + try!(self.name.to_css(dest)); try!(dest.write_str(" { ")); let iter = self.keyframes.iter(); let mut first = true; @@ -922,7 +923,7 @@ enum AtRulePrelude { /// A @viewport rule prelude. Viewport, /// A @keyframes rule, with its animation name and vendor prefix if exists. - Keyframes(Atom, Option), + Keyframes(CustomIdent, Option), /// A @page rule prelude. Page, } @@ -1123,12 +1124,12 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { return Err(()) } let name = match input.next() { - Ok(Token::Ident(ref value)) if value != "none" => Atom::from(&**value), - Ok(Token::QuotedString(value)) => Atom::from(&*value), + Ok(Token::Ident(value)) => CustomIdent::from_ident(value, &["none"])?, + Ok(Token::QuotedString(value)) => CustomIdent(Atom::from(value)), _ => return Err(()) }; - Ok(AtRuleType::WithBlock(AtRulePrelude::Keyframes(Atom::from(name), prefix))) + Ok(AtRuleType::WithBlock(AtRulePrelude::Keyframes(name, prefix))) }, "page" => { if cfg!(feature = "gecko") { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 38dbe24b4c1..4f5489f0d8c 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -349,13 +349,13 @@ impl Stylist { // Don't let a prefixed keyframes animation override a non-prefixed one. let needs_insertion = keyframes_rule.vendor_prefix.is_none() || - self.animations.get(&keyframes_rule.name).map_or(true, |rule| + self.animations.get(&keyframes_rule.name.0).map_or(true, |rule| rule.vendor_prefix.is_some()); if needs_insertion { let animation = KeyframesAnimation::from_keyframes( &keyframes_rule.keyframes, keyframes_rule.vendor_prefix.clone(), guard); debug!("Found valid keyframe animation: {:?}", animation); - self.animations.insert(keyframes_rule.name.clone(), animation); + self.animations.insert(keyframes_rule.name.0.clone(), animation); } } CssRule::FontFace(ref rule) => { From 627c823d0aaf1fdf9bf28b6cd9c38454e0b6ab2a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 05:53:37 +0200 Subject: [PATCH 03/26] Use CustomIdent in counter-increment --- components/layout/generated_content.rs | 6 ++- components/style/properties/gecko.mako.rs | 6 +-- .../properties/longhand/counters.mako.rs | 43 ++++++++----------- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs index 887562e89fb..a23fd165ecf 100644 --- a/components/layout/generated_content.rs +++ b/components/layout/generated_content.rs @@ -273,6 +273,7 @@ impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> { self.traversal.list_item.truncate_to_level(self.level); for &(ref counter_name, value) in &fragment.style().get_counters().counter_reset.0 { + let counter_name = &*counter_name.0; if let Some(ref mut counter) = self.traversal.counters.get_mut(counter_name) { counter.reset(self.level, value); continue @@ -280,10 +281,11 @@ impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> { let mut counter = Counter::new(); counter.reset(self.level, value); - self.traversal.counters.insert((*counter_name).clone(), counter); + self.traversal.counters.insert(counter_name.to_owned(), counter); } for &(ref counter_name, value) in &fragment.style().get_counters().counter_increment.0 { + let counter_name = &*counter_name.0; if let Some(ref mut counter) = self.traversal.counters.get_mut(counter_name) { counter.increment(self.level, value); continue @@ -291,7 +293,7 @@ impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> { let mut counter = Counter::new(); counter.increment(self.level, value); - self.traversal.counters.insert((*counter_name).clone(), counter); + self.traversal.counters.insert(counter_name.to_owned(), counter); } self.incremented = true diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index ee419a9bf77..b8bf35e04c7 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -4012,9 +4012,9 @@ clip-path unsafe { bindings::Gecko_ClearAndResizeCounter${counter_property}s(&mut self.gecko, v.0.len() as u32); - for (i, item) in v.0.into_iter().enumerate() { - self.gecko.m${counter_property}s[i].mCounter.assign_utf8(&item.0); - self.gecko.m${counter_property}s[i].mValue = item.1; + for (i, (name, value)) in v.0.into_iter().enumerate() { + self.gecko.m${counter_property}s[i].mCounter.assign(name.0.as_slice()); + self.gecko.m${counter_property}s[i].mValue = value; } } } diff --git a/components/style/properties/longhand/counters.mako.rs b/components/style/properties/longhand/counters.mako.rs index c8dc8ebb4ce..d5e68d72970 100644 --- a/components/style/properties/longhand/counters.mako.rs +++ b/components/style/properties/longhand/counters.mako.rs @@ -240,21 +240,22 @@ use std::fmt; use style_traits::ToCss; use super::content; - use values::HasViewportPercentage; + use values::{HasViewportPercentage, CustomIdent}; use cssparser::{Token, serialize_identifier}; use std::borrow::{Cow, ToOwned}; #[derive(Debug, Clone, PartialEq)] - pub struct SpecifiedValue(pub Vec<(String, specified::Integer)>); + pub struct SpecifiedValue(pub Vec<(CustomIdent, specified::Integer)>); pub mod computed_value { use std::fmt; use style_traits::ToCss; + use values::CustomIdent; #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct T(pub Vec<(String, i32)>); + pub struct T(pub Vec<(CustomIdent, i32)>); impl ToCss for T { fn to_css(&self, dest: &mut W) -> fmt::Result @@ -266,14 +267,14 @@ } let mut first = true; - for pair in &self.0 { + for &(ref name, value) in &self.0 { if !first { - try!(dest.write_str(" ")); + dest.write_str(" ")?; } first = false; - try!(serialize_identifier(&pair.0, dest)); - try!(dest.write_str(" ")); - try!(pair.1.to_css(dest)); + name.to_css(dest)?; + dest.write_str(" ")?; + value.to_css(dest)?; } Ok(()) } @@ -284,14 +285,14 @@ type ComputedValue = computed_value::T; fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { - computed_value::T(self.0.iter().map(|entry| { - (entry.0.clone(), entry.1.to_computed_value(context)) + computed_value::T(self.0.iter().map(|&(ref name, ref value)| { + (name.clone(), value.to_computed_value(context)) }).collect::>()) } fn from_computed_value(computed: &Self::ComputedValue) -> Self { - SpecifiedValue(computed.0.iter().map(|entry| { - (entry.0.clone(), specified::Integer::from_computed_value(&entry.1)) + SpecifiedValue(computed.0.iter().map(|&(ref name, ref value)| { + (name.clone(), specified::Integer::from_computed_value(&value)) }).collect::>()) } } @@ -311,14 +312,14 @@ return dest.write_str("none"); } let mut first = true; - for pair in &self.0 { + for &(ref name, ref value) in &self.0 { if !first { - try!(dest.write_str(" ")); + dest.write_str(" ")?; } first = false; - try!(serialize_identifier(&pair.0, dest)); - try!(dest.write_str(" ")); - try!(pair.1.to_css(dest)); + name.to_css(dest)?; + dest.write_str(" ")?; + value.to_css(dest)?; } Ok(()) @@ -339,13 +340,7 @@ let mut counters = Vec::new(); loop { let counter_name = match input.next() { - Ok(Token::Ident(ident)) => { - if CSSWideKeyword::from_ident(&ident).is_some() || ident.eq_ignore_ascii_case("none") { - // Don't accept CSS-wide keywords or none as the counter name. - return Err(()); - } - (*ident).to_owned() - } + Ok(Token::Ident(ident)) => CustomIdent::from_ident(ident, &["none"])?, Ok(_) => return Err(()), Err(_) => break, }; From 71f9a0c8488b77b7ab7d88fb384c311ba01c41c2 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 12:35:50 +0200 Subject: [PATCH 04/26] Fix up unit tests --- tests/unit/style/parsing/animation.rs | 11 ++++++----- tests/unit/style/properties/serialization.rs | 5 +++-- tests/unit/style/stylesheets.rs | 3 ++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/unit/style/parsing/animation.rs b/tests/unit/style/parsing/animation.rs index 3db81be6484..936d6c644f7 100644 --- a/tests/unit/style/parsing/animation.rs +++ b/tests/unit/style/parsing/animation.rs @@ -7,6 +7,7 @@ use servo_atoms::Atom; use style::parser::Parse; use style::properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount; use style::properties::longhands::animation_name; +use style::values::CustomIdent; use style_traits::ToCss; #[test] @@ -14,13 +15,13 @@ fn test_animation_name() { use self::animation_name::single_value::SpecifiedValue as SingleValue; let other_name = Atom::from("other-name"); assert_eq!(parse_longhand!(animation_name, "none"), - animation_name::SpecifiedValue(vec![SingleValue(atom!(""))])); + animation_name::SpecifiedValue(vec![SingleValue(CustomIdent(atom!("")))])); assert_eq!(parse_longhand!(animation_name, "other-name, none, 'other-name', \"other-name\""), animation_name::SpecifiedValue( - vec![SingleValue(other_name.clone()), - SingleValue(atom!("")), - SingleValue(other_name.clone()), - SingleValue(other_name.clone())])); + vec![SingleValue(CustomIdent(other_name.clone())), + SingleValue(CustomIdent(atom!(""))), + SingleValue(CustomIdent(other_name.clone())), + SingleValue(CustomIdent(other_name.clone()))])); } #[test] diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index aeb07f87484..fea8d3f416e 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -8,6 +8,7 @@ use style::properties::{PropertyDeclaration, Importance, PropertyId}; use style::properties::longhands::outline_color::computed_value::T as ComputedColor; use style::properties::parse_property_declaration_list; use style::values::{RGBA, Auto}; +use style::values::CustomIdent; use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCalcLength}; use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent}; use style::values::specified::url::SpecifiedUrl; @@ -1259,8 +1260,8 @@ mod shorthand_serialization { fn counter_increment_with_properties_should_serialize_correctly() { let mut properties = Vec::new(); - properties.push(("counter1".to_owned(), Integer::new(1))); - properties.push(("counter2".to_owned(), Integer::new(-4))); + properties.push((CustomIdent("counter1".into()), Integer::new(1))); + properties.push((CustomIdent("counter2".into()), Integer::new(-4))); let counter_increment = CounterIncrement(properties); let counter_increment_css = "counter1 1 counter2 -4"; diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 8005139e676..9a56ee80d53 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -23,6 +23,7 @@ use style::properties::longhands::animation_play_state; use style::shared_lock::SharedRwLock; use style::stylesheets::{Origin, Namespaces}; use style::stylesheets::{Stylesheet, NamespaceRule, CssRule, CssRules, StyleRule, KeyframesRule}; +use style::values::CustomIdent; use style::values::specified::{LengthOrPercentageOrAuto, Percentage}; pub fn block_from(iterable: I) -> PropertyDeclarationBlock @@ -221,7 +222,7 @@ fn test_parse_stylesheet() { ]))), }))), CssRule::Keyframes(Arc::new(stylesheet.shared_lock.wrap(KeyframesRule { - name: "foo".into(), + name: CustomIdent("foo".into()), keyframes: vec![ Arc::new(stylesheet.shared_lock.wrap(Keyframe { selector: KeyframeSelector::new_for_unit_testing( From 5a8e3308c1c980a645246504bbb0f145f800a749 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 05:58:36 +0200 Subject: [PATCH 05/26] Remove some intermediate conversions in style/properties/gecko.mako.rs --- components/style/properties/gecko.mako.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index b8bf35e04c7..b74bb9e8cd3 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1175,11 +1175,10 @@ fn static_assert() { % for value in GRID_LINES: pub fn set_${value.name}(&mut self, v: longhands::${value.name}::computed_value::T) { - use nsstring::nsCString; use gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine}; let ident = v.ident.unwrap_or(String::new()); - self.gecko.${value.gecko}.mLineName.assign_utf8(&nsCString::from(&*ident)); + self.gecko.${value.gecko}.mLineName.assign_utf8(&ident); self.gecko.${value.gecko}.mHasSpan = v.is_span; self.gecko.${value.gecko}.mInteger = v.integer.map(|i| { // clamping the integer between a range @@ -2197,15 +2196,13 @@ fn static_assert() { } pub fn set_animation_name(&mut self, v: longhands::animation_name::computed_value::T) { - use nsstring::nsCString; - debug_assert!(!v.0.is_empty()); unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) }; self.gecko.mAnimationNameCount = v.0.len() as u32; for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) { // TODO This is inefficient. We should fix this in bug 1329169. - gecko.mName.assign_utf8(&nsCString::from(servo.0 .0.to_string())); + gecko.mName.assign(servo.0 .0.as_slice()); } } pub fn animation_name_at(&self, index: usize) @@ -2887,15 +2884,14 @@ fn static_assert() { pub fn set_quotes(&mut self, other: longhands::quotes::computed_value::T) { use gecko_bindings::bindings::Gecko_NewStyleQuoteValues; use gecko_bindings::sugar::refptr::UniqueRefPtr; - use nsstring::nsCString; let mut refptr = unsafe { UniqueRefPtr::from_addrefed(Gecko_NewStyleQuoteValues(other.0.len() as u32)) }; for (servo, gecko) in other.0.into_iter().zip(refptr.mQuotePairs.iter_mut()) { - gecko.first.assign_utf8(&nsCString::from(&*servo.0)); - gecko.second.assign_utf8(&nsCString::from(&*servo.1)); + gecko.first.assign_utf8(&servo.0); + gecko.second.assign_utf8(&servo.1); } unsafe { self.gecko.mQuotes.set_move(refptr.get()) } @@ -3382,7 +3378,6 @@ fn static_assert() { <%call expr="impl_simple_copy('text_emphasis_position', 'mTextEmphasisPosition')"> pub fn set_text_emphasis_style(&mut self, v: longhands::text_emphasis_style::computed_value::T) { - use nsstring::nsCString; use properties::longhands::text_emphasis_style::computed_value::T; use properties::longhands::text_emphasis_style::ShapeKeyword; @@ -3409,7 +3404,7 @@ fn static_assert() { (structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING, &**s) }, }; - self.gecko.mTextEmphasisStyleString.assign_utf8(&nsCString::from(s)); + self.gecko.mTextEmphasisStyleString.assign_utf8(s); self.gecko.mTextEmphasisStyle = te as u8; } @@ -3490,12 +3485,11 @@ fn static_assert() { use properties::longhands::text_overflow::{SpecifiedValue, Side}; fn set(side: &mut nsStyleTextOverflowSide, value: &Side) { - use nsstring::nsCString; let ty = match *value { Side::Clip => structs::NS_STYLE_TEXT_OVERFLOW_CLIP, Side::Ellipsis => structs::NS_STYLE_TEXT_OVERFLOW_ELLIPSIS, Side::String(ref s) => { - side.mString.assign_utf8(&nsCString::from(&**s)); + side.mString.assign_utf8(s); structs::NS_STYLE_TEXT_OVERFLOW_STRING } }; From 797f40b0a30013a4db5c3ae341ad17c5b380f4d6 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 07:38:54 +0200 Subject: [PATCH 06/26] Add initial style system support for @counter-style rules --- components/script/dom/cssrule.rs | 1 + components/style/counter_style.rs | 150 ++++++++++++++++++++++++++++++ components/style/gecko/rules.rs | 15 ++- components/style/lib.rs | 1 + components/style/stylesheets.rs | 35 +++++-- 5 files changed, 194 insertions(+), 8 deletions(-) create mode 100644 components/style/counter_style.rs diff --git a/components/script/dom/cssrule.rs b/components/script/dom/cssrule.rs index 1a24d5839bf..6bfe057d6d9 100644 --- a/components/script/dom/cssrule.rs +++ b/components/script/dom/cssrule.rs @@ -78,6 +78,7 @@ impl CSSRule { StyleCssRule::Import(s) => Root::upcast(CSSImportRule::new(window, parent_stylesheet, s)), StyleCssRule::Style(s) => Root::upcast(CSSStyleRule::new(window, parent_stylesheet, s)), StyleCssRule::FontFace(s) => Root::upcast(CSSFontFaceRule::new(window, parent_stylesheet, s)), + StyleCssRule::CounterStyle(_) => unimplemented!(), StyleCssRule::Keyframes(s) => Root::upcast(CSSKeyframesRule::new(window, parent_stylesheet, s)), StyleCssRule::Media(s) => Root::upcast(CSSMediaRule::new(window, parent_stylesheet, s)), StyleCssRule::Namespace(s) => Root::upcast(CSSNamespaceRule::new(window, parent_stylesheet, s)), diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs new file mode 100644 index 00000000000..a78a64f31ec --- /dev/null +++ b/components/style/counter_style.rs @@ -0,0 +1,150 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! The [`@counter-style`][counter-style] at-rule. +//! +//! [counter-style]: https://drafts.csswg.org/css-counter-styles/ + +use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser}; +#[cfg(feature = "gecko")] use gecko::rules::CounterStyleDescriptors; +#[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSCounterDesc; +use parser::{ParserContext, log_css_error, Parse}; +use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard}; +use std::fmt; +use style_traits::ToCss; +use values::CustomIdent; + +/// Parse the body (inside `{}`) of an @counter-style rule +pub fn parse_counter_style_body(name: CustomIdent, context: &ParserContext, input: &mut Parser) + -> Result { + let mut rule = CounterStyleRule::initial(name); + { + let parser = CounterStyleRuleParser { + context: context, + rule: &mut rule, + }; + let mut iter = DeclarationListParser::new(input, parser); + while let Some(declaration) = iter.next() { + if let Err(range) = declaration { + let pos = range.start; + let message = format!("Unsupported @counter-style descriptor declaration: '{}'", + iter.input.slice(range)); + log_css_error(iter.input, pos, &*message, context); + } + } + } + Ok(rule) +} + +struct CounterStyleRuleParser<'a, 'b: 'a> { + context: &'a ParserContext<'b>, + rule: &'a mut CounterStyleRule, +} + +/// Default methods reject all at rules. +impl<'a, 'b> AtRuleParser for CounterStyleRuleParser<'a, 'b> { + type Prelude = (); + type AtRule = (); +} + + +macro_rules! counter_style_descriptors { + ( + $( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty = $initial: expr, )+ + ) => { + /// An @counter-style rule + #[derive(Debug)] + pub struct CounterStyleRule { + name: CustomIdent, + $( + #[$doc] + $ident: $ty, + )+ + } + + impl CounterStyleRule { + fn initial(name: CustomIdent) -> Self { + CounterStyleRule { + name: name, + $( + $ident: $initial, + )+ + } + } + + /// Convert to Gecko types + #[cfg(feature = "gecko")] + pub fn set_descriptors(&self, descriptors: &mut CounterStyleDescriptors) { + $( + descriptors[nsCSSCounterDesc::$gecko_ident as usize].set_from(&self.$ident) + )* + } + } + + impl<'a, 'b> DeclarationParser for CounterStyleRuleParser<'a, 'b> { + type Declaration = (); + + fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> { + match_ignore_ascii_case! { name, + $( + $name => { + // DeclarationParser also calls parse_entirely + // so we’d normally not need to, + // but in this case we do because we set the value as a side effect + // rather than returning it. + let value = input.parse_entirely(|i| Parse::parse(self.context, i))?; + self.rule.$ident = value + } + )* + _ => return Err(()) + } + Ok(()) + } + } + + impl ToCssWithGuard for CounterStyleRule { + fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result + where W: fmt::Write { + dest.write_str("@counter-style ")?; + self.name.to_css(dest)?; + dest.write_str(" {\n")?; + $( + dest.write_str(concat!(" ", $name, ": "))?; + ToCss::to_css(&self.$ident, dest)?; + dest.write_str(";\n")?; + )+ + dest.write_str("}") + } + } + } +} + +counter_style_descriptors! { + /// The algorithm for constructing a string representation of a counter value + "system" system / eCSSCounterDesc_System: System = System::Symbolic, +} + +/// Value of the 'system' descriptor +#[derive(Debug)] +pub enum System { + /// Cycles through provided symbols, doubling, tripling, etc. + Symbolic, +} + +impl Parse for System { + fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + match_ignore_ascii_case! { &input.expect_ident()?, + "symbolic" => Ok(System::Symbolic), + _ => Err(()) + } + } +} + +impl ToCss for System { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + System::Symbolic => dest.write_str("symbolic") + } + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 548266c4c40..0dced7dea56 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -6,10 +6,11 @@ use computed_values::{font_style, font_weight, font_stretch}; use computed_values::font_family::FamilyName; +use counter_style::System; use cssparser::UnicodeRange; use font_face::{FontFaceRuleData, Source}; use gecko_bindings::bindings; -use gecko_bindings::structs::{self, nsCSSFontFaceRule, nsCSSValue}; +use gecko_bindings::structs::{self, nsCSSFontFaceRule, nsCSSValue, nsCSSCounterDesc}; use gecko_bindings::sugar::ns_css_value::ToNsCssValue; use gecko_bindings::sugar::refptr::{RefPtr, UniqueRefPtr}; use shared_lock::{ToCssWithGuard, SharedRwLockReadGuard}; @@ -133,3 +134,15 @@ impl ToCssWithGuard for FontFaceRule { write!(dest, "{}", css_text) } } + +/// The type of nsCSSCounterStyleRule::mValues +pub type CounterStyleDescriptors = [nsCSSValue; nsCSSCounterDesc::eCSSCounterDesc_COUNT as usize]; + +impl ToNsCssValue for System { + fn convert(&self, v: &mut nsCSSValue) { + match *self { + System::Symbolic => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_SYMBOLIC as i32), + } + } +} + diff --git a/components/style/lib.rs b/components/style/lib.rs index 74f5cc5d4b6..7479279418d 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -89,6 +89,7 @@ pub mod bloom; pub mod cache; pub mod cascade_info; pub mod context; +pub mod counter_style; pub mod custom_properties; pub mod data; pub mod dom; diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 90f551cfdf8..7429370a458 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -7,6 +7,7 @@ #![deny(missing_docs)] use {Atom, Prefix, Namespace}; +use counter_style::{CounterStyleRule, parse_counter_style_body}; use cssparser::{AtRuleParser, Parser, QualifiedRuleParser}; use cssparser::{AtRuleType, RuleListParser, Token, parse_one_rule}; use cssparser::ToCss as ParserToCss; @@ -290,6 +291,7 @@ pub enum CssRule { Style(Arc>), Media(Arc>), FontFace(Arc>), + CounterStyle(Arc>), Viewport(Arc>), Keyframes(Arc>), Supports(Arc>), @@ -332,15 +334,16 @@ impl CssRule { #[allow(missing_docs)] pub fn rule_type(&self) -> CssRuleType { match *self { - CssRule::Style(_) => CssRuleType::Style, - CssRule::Import(_) => CssRuleType::Import, - CssRule::Media(_) => CssRuleType::Media, - CssRule::FontFace(_) => CssRuleType::FontFace, + CssRule::Style(_) => CssRuleType::Style, + CssRule::Import(_) => CssRuleType::Import, + CssRule::Media(_) => CssRuleType::Media, + CssRule::FontFace(_) => CssRuleType::FontFace, + CssRule::CounterStyle(_) => CssRuleType::CounterStyle, CssRule::Keyframes(_) => CssRuleType::Keyframes, CssRule::Namespace(_) => CssRuleType::Namespace, - CssRule::Viewport(_) => CssRuleType::Viewport, - CssRule::Supports(_) => CssRuleType::Supports, - CssRule::Page(_) => CssRuleType::Page, + CssRule::Viewport(_) => CssRuleType::Viewport, + CssRule::Supports(_) => CssRuleType::Supports, + CssRule::Page(_) => CssRuleType::Page, } } @@ -373,6 +376,7 @@ impl CssRule { CssRule::Namespace(_) | CssRule::Style(_) | CssRule::FontFace(_) | + CssRule::CounterStyle(_) | CssRule::Viewport(_) | CssRule::Keyframes(_) | CssRule::Page(_) => { @@ -446,6 +450,7 @@ impl ToCssWithGuard for CssRule { CssRule::Import(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::Style(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::FontFace(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::CounterStyle(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::Viewport(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::Keyframes(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::Media(ref lock) => lock.read_with(guard).to_css(guard, dest), @@ -835,6 +840,7 @@ rule_filter! { effective_style_rules(Style => StyleRule), effective_media_rules(Media => MediaRule), effective_font_face_rules(FontFace => FontFaceRule), + effective_counter_style_rules(CounterStyle => CounterStyleRule), effective_viewport_rules(Viewport => ViewportRule), effective_keyframes_rules(Keyframes => KeyframesRule), effective_supports_rules(Supports => SupportsRule), @@ -916,6 +922,8 @@ pub enum VendorPrefix { enum AtRulePrelude { /// A @font-face rule prelude. FontFace, + /// A @counter-style rule prelude, with its counter style name. + CounterStyle(CustomIdent), /// A @media rule prelude, with its media queries. Media(Arc>), /// An @supports rule, with its conditional @@ -1103,6 +1111,14 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { "font-face" => { Ok(AtRuleType::WithBlock(AtRulePrelude::FontFace)) }, + "counter-style" => { + if !cfg!(feature = "gecko") { + // Support for this rule is not fully implemented in Servo yet. + return Err(()) + } + let name = CustomIdent::from_ident(input.expect_ident()?, &["decimal", "none"])?; + Ok(AtRuleType::WithBlock(AtRulePrelude::CounterStyle(name))) + }, "viewport" => { if is_viewport_enabled() { Ok(AtRuleType::WithBlock(AtRulePrelude::Viewport)) @@ -1149,6 +1165,11 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { Ok(CssRule::FontFace(Arc::new(self.shared_lock.wrap( parse_font_face_block(&context, input).into())))) } + AtRulePrelude::CounterStyle(name) => { + let context = ParserContext::new_with_rule_type(self.context, Some(CssRuleType::CounterStyle)); + Ok(CssRule::CounterStyle(Arc::new(self.shared_lock.wrap( + parse_counter_style_body(name, &context, input)?)))) + } AtRulePrelude::Media(media_queries) => { Ok(CssRule::Media(Arc::new(self.shared_lock.wrap(MediaRule { media_queries: media_queries, From 4477a2da40c1a6c884e808252f3bddc38357a7a0 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 08:09:27 +0200 Subject: [PATCH 07/26] Add the 'system' descriptor of @counter-style --- components/style/counter_style.rs | 51 +++++++++++++++++++++++++++++-- components/style/gecko/rules.rs | 12 ++++++++ components/style/stylesheets.rs | 4 +-- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index a78a64f31ec..19290f8ea3b 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -15,6 +15,11 @@ use std::fmt; use style_traits::ToCss; use values::CustomIdent; +/// Parse the prelude of an @counter-style rule +pub fn parse_counter_style_name(input: &mut Parser) -> Result { + CustomIdent::from_ident(input.expect_ident()?, &["decimal", "none"]) +} + /// Parse the body (inside `{}`) of an @counter-style rule pub fn parse_counter_style_body(name: CustomIdent, context: &ParserContext, input: &mut Parser) -> Result { @@ -128,14 +133,41 @@ counter_style_descriptors! { /// Value of the 'system' descriptor #[derive(Debug)] pub enum System { - /// Cycles through provided symbols, doubling, tripling, etc. + /// 'cyclic' + Cyclic, + /// 'numeric' + Numeric, + /// 'alphabetic' + Alphabetic, + /// 'symbolic' Symbolic, + /// 'additive' + Additive, + /// 'fixed ?' + Fixed { + /// '?' + first_symbol_value: Option + }, + /// 'extends ' + Extends(CustomIdent), } impl Parse for System { fn parse(_context: &ParserContext, input: &mut Parser) -> Result { match_ignore_ascii_case! { &input.expect_ident()?, + "cyclic" => Ok(System::Cyclic), + "numeric" => Ok(System::Numeric), + "alphabetic" => Ok(System::Alphabetic), "symbolic" => Ok(System::Symbolic), + "additive" => Ok(System::Additive), + "fixed" => { + let first_symbol_value = input.try(|i| i.expect_integer()).ok(); + Ok(System::Fixed { first_symbol_value: first_symbol_value }) + } + "extends" => { + let other = parse_counter_style_name(input)?; + Ok(System::Extends(other)) + } _ => Err(()) } } @@ -144,7 +176,22 @@ impl Parse for System { impl ToCss for System { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { match *self { - System::Symbolic => dest.write_str("symbolic") + System::Cyclic => dest.write_str("cyclic"), + System::Numeric => dest.write_str("numeric"), + System::Alphabetic => dest.write_str("alphabetic"), + System::Symbolic => dest.write_str("symbolic"), + System::Additive => dest.write_str("additive"), + System::Fixed { first_symbol_value } => { + if let Some(value) = first_symbol_value { + write!(dest, "fixed {}", value) + } else { + dest.write_str("fixed") + } + } + System::Extends(ref other) => { + dest.write_str("extends ")?; + other.to_css(dest) + } } } } diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 0dced7dea56..086600059bb 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -141,7 +141,19 @@ pub type CounterStyleDescriptors = [nsCSSValue; nsCSSCounterDesc::eCSSCounterDes impl ToNsCssValue for System { fn convert(&self, v: &mut nsCSSValue) { match *self { + System::Cyclic => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_CYCLIC as i32), + System::Numeric => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_NUMERIC as i32), + System::Alphabetic => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_ALPHABETIC as i32), System::Symbolic => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_SYMBOLIC as i32), + System::Additive => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_ADDITIVE as i32), + System::Fixed { first_symbol_value: _ } => { + // FIXME: add bindings for nsCSSValue::SetPairValue or equivalent + unimplemented!() + } + System::Extends(ref _other) => { + // FIXME: add bindings for nsCSSValue::SetPairValue or equivalent + unimplemented!() + } } } } diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 7429370a458..bf7c10178f0 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -7,7 +7,7 @@ #![deny(missing_docs)] use {Atom, Prefix, Namespace}; -use counter_style::{CounterStyleRule, parse_counter_style_body}; +use counter_style::{CounterStyleRule, parse_counter_style_name, parse_counter_style_body}; use cssparser::{AtRuleParser, Parser, QualifiedRuleParser}; use cssparser::{AtRuleType, RuleListParser, Token, parse_one_rule}; use cssparser::ToCss as ParserToCss; @@ -1116,7 +1116,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { // Support for this rule is not fully implemented in Servo yet. return Err(()) } - let name = CustomIdent::from_ident(input.expect_ident()?, &["decimal", "none"])?; + let name = parse_counter_style_name(input)?; Ok(AtRuleType::WithBlock(AtRulePrelude::CounterStyle(name))) }, "viewport" => { From d1558a2025feae49d252e1e39a6bdec66332f7bf Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 09:10:31 +0200 Subject: [PATCH 08/26] Add 'negative' descriptor of @counter-style --- components/style/counter_style.rs | 72 ++++++++++++++++++++++++++++--- components/style/gecko/rules.rs | 57 ++++++++++++++++++------ 2 files changed, 109 insertions(+), 20 deletions(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 19290f8ea3b..9d3551a4d52 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -6,7 +6,8 @@ //! //! [counter-style]: https://drafts.csswg.org/css-counter-styles/ -use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser}; +use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser, Token}; +use cssparser::{serialize_string, serialize_identifier}; #[cfg(feature = "gecko")] use gecko::rules::CounterStyleDescriptors; #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSCounterDesc; use parser::{ParserContext, log_css_error, Parse}; @@ -56,7 +57,7 @@ impl<'a, 'b> AtRuleParser for CounterStyleRuleParser<'a, 'b> { macro_rules! counter_style_descriptors { ( - $( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty = $initial: expr, )+ + $( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty = $initial: expr; )+ ) => { /// An @counter-style rule #[derive(Debug)] @@ -82,7 +83,7 @@ macro_rules! counter_style_descriptors { #[cfg(feature = "gecko")] pub fn set_descriptors(&self, descriptors: &mut CounterStyleDescriptors) { $( - descriptors[nsCSSCounterDesc::$gecko_ident as usize].set_from(&self.$ident) + descriptors[nsCSSCounterDesc::$gecko_ident as usize].set_from(&self.$ident); )* } } @@ -126,11 +127,15 @@ macro_rules! counter_style_descriptors { } counter_style_descriptors! { - /// The algorithm for constructing a string representation of a counter value - "system" system / eCSSCounterDesc_System: System = System::Symbolic, + /// https://drafts.csswg.org/css-counter-styles/#counter-style-system + "system" system / eCSSCounterDesc_System: System = System::Symbolic; + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-negative + "negative" negative / eCSSCounterDesc_Negative: Negative = + Negative(Symbol::String("-".to_owned()), None); } -/// Value of the 'system' descriptor +/// https://drafts.csswg.org/css-counter-styles/#counter-style-system #[derive(Debug)] pub enum System { /// 'cyclic' @@ -195,3 +200,58 @@ impl ToCss for System { } } } + +/// https://drafts.csswg.org/css-counter-styles/#typedef-symbol +#[derive(Debug)] +pub enum Symbol { + /// + String(String), + /// + Ident(String), + // Not implemented: + // /// + // Image(Image), +} + +impl Parse for Symbol { + fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + match input.next() { + Ok(Token::QuotedString(s)) => Ok(Symbol::String(s.into_owned())), + Ok(Token::Ident(s)) => Ok(Symbol::Ident(s.into_owned())), + _ => Err(()) + } + } +} + +impl ToCss for Symbol { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + Symbol::String(ref s) => serialize_string(s, dest), + Symbol::Ident(ref s) => serialize_identifier(s, dest), + } + } +} + +/// https://drafts.csswg.org/css-counter-styles/#counter-style-negative +#[derive(Debug)] +pub struct Negative(pub Symbol, pub Option); + +impl Parse for Negative { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + Ok(Negative( + Symbol::parse(context, input)?, + input.try(|input| Symbol::parse(context, input)).ok(), + )) + } +} + +impl ToCss for Negative { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + self.0.to_css(dest)?; + if let Some(ref symbol) = self.1 { + dest.write_char(' ')?; + symbol.to_css(dest)? + } + Ok(()) + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 086600059bb..068696b1cdf 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -6,7 +6,7 @@ use computed_values::{font_style, font_weight, font_stretch}; use computed_values::font_family::FamilyName; -use counter_style::System; +use counter_style; use cssparser::UnicodeRange; use font_face::{FontFaceRuleData, Source}; use gecko_bindings::bindings; @@ -138,23 +138,52 @@ impl ToCssWithGuard for FontFaceRule { /// The type of nsCSSCounterStyleRule::mValues pub type CounterStyleDescriptors = [nsCSSValue; nsCSSCounterDesc::eCSSCounterDesc_COUNT as usize]; -impl ToNsCssValue for System { - fn convert(&self, v: &mut nsCSSValue) { +impl ToNsCssValue for counter_style::System { + fn convert(&self, nscssvalue: &mut nsCSSValue) { + use counter_style::System::*; match *self { - System::Cyclic => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_CYCLIC as i32), - System::Numeric => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_NUMERIC as i32), - System::Alphabetic => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_ALPHABETIC as i32), - System::Symbolic => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_SYMBOLIC as i32), - System::Additive => v.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_ADDITIVE as i32), - System::Fixed { first_symbol_value: _ } => { - // FIXME: add bindings for nsCSSValue::SetPairValue or equivalent - unimplemented!() + Cyclic => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_CYCLIC as i32), + Numeric => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_NUMERIC as i32), + Alphabetic => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_ALPHABETIC as i32), + Symbolic => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_SYMBOLIC as i32), + Additive => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_ADDITIVE as i32), + Fixed { first_symbol_value } => { + let mut a = nsCSSValue::null(); + let mut b = nsCSSValue::null(); + a.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_FIXED as i32); + b.set_integer(first_symbol_value.unwrap_or(1)); + //nscssvalue.set_pair(a, b); // FIXME: add bindings for nsCSSValue::SetPairValue } - System::Extends(ref _other) => { - // FIXME: add bindings for nsCSSValue::SetPairValue or equivalent - unimplemented!() + Extends(ref other) => { + let mut a = nsCSSValue::null(); + let mut b = nsCSSValue::null(); + a.set_enum(structs::NS_STYLE_COUNTER_SYSTEM_EXTENDS as i32); + b.set_string_from_atom(&other.0); + //nscssvalue.set_pair(a, b); // FIXME: add bindings for nsCSSValue::SetPairValue } } } } +impl ToNsCssValue for counter_style::Negative { + fn convert(&self, nscssvalue: &mut nsCSSValue) { + if let Some(ref second) = self.1 { + let mut a = nsCSSValue::null(); + let mut b = nsCSSValue::null(); + a.set_from(&self.0); + b.set_from(second); + //nscssvalue.set_pair(a, b); // FIXME: add bindings for nsCSSValue::SetPairValue + } else { + nscssvalue.set_from(&self.0) + } + } +} + +impl ToNsCssValue for counter_style::Symbol { + fn convert(&self, nscssvalue: &mut nsCSSValue) { + match *self { + counter_style::Symbol::String(ref s) => nscssvalue.set_string(s), + counter_style::Symbol::Ident(ref s) => nscssvalue.set_ident(s), + } + } +} From 29bcb5b6368adab46fc31b4bc190e3b0b551baa2 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 09:13:15 +0200 Subject: [PATCH 09/26] Add 'prefix' and 'suffix' descriptors for @counter-style --- components/style/counter_style.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 9d3551a4d52..4cca58f75d7 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -133,6 +133,12 @@ counter_style_descriptors! { /// https://drafts.csswg.org/css-counter-styles/#counter-style-negative "negative" negative / eCSSCounterDesc_Negative: Negative = Negative(Symbol::String("-".to_owned()), None); + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-prefix + "prefix" prefix / eCSSCounterDesc_Prefix: Symbol = Symbol::String("".to_owned()); + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-suffix + "suffix" suffix / eCSSCounterDesc_Suffix: Symbol = Symbol::String(". ".to_owned()); } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system From 6f7968446889cf9f39f146d306cd80b421eeecce Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 09:45:44 +0200 Subject: [PATCH 10/26] Add 'range' descriptor to @counter-style --- components/style/counter_style.rs | 70 +++++++++++++++++++++++++++++++ components/style/gecko/rules.rs | 23 ++++++++++ 2 files changed, 93 insertions(+) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 4cca58f75d7..ad8897e99c2 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -12,7 +12,9 @@ use cssparser::{serialize_string, serialize_identifier}; #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSCounterDesc; use parser::{ParserContext, log_css_error, Parse}; use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard}; +use std::ascii::AsciiExt; use std::fmt; +use std::ops::Range; use style_traits::ToCss; use values::CustomIdent; @@ -139,6 +141,10 @@ counter_style_descriptors! { /// https://drafts.csswg.org/css-counter-styles/#counter-style-suffix "suffix" suffix / eCSSCounterDesc_Suffix: Symbol = Symbol::String(". ".to_owned()); + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-range + "range" range / eCSSCounterDesc_Range: Ranges = + Ranges(Vec::new()); // Empty Vec represents 'auto' } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system @@ -261,3 +267,67 @@ impl ToCss for Negative { Ok(()) } } + +/// https://drafts.csswg.org/css-counter-styles/#counter-style-range +/// +/// Empty Vec represents 'auto' +#[derive(Debug)] +pub struct Ranges(pub Vec>>); + +impl Parse for Ranges { + fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + if input.try(|input| input.expect_ident_matching("auto")).is_ok() { + Ok(Ranges(Vec::new())) + } else { + input.parse_comma_separated(|input| { + let opt_start = parse_bound(input)?; + let opt_end = parse_bound(input)?; + if let (Some(start), Some(end)) = (opt_start, opt_end) { + if start > end { + return Err(()) + } + } + Ok(opt_start..opt_end) + }).map(Ranges) + } + } +} + +fn parse_bound(input: &mut Parser) -> Result, ()> { + match input.next() { + Ok(Token::Number(ref v)) if v.int_value.is_some() => Ok(Some(v.int_value.unwrap())), + Ok(Token::Ident(ref ident)) if ident.eq_ignore_ascii_case("infinite") => Ok(None), + _ => Err(()) + } +} + +impl ToCss for Ranges { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + let mut iter = self.0.iter(); + if let Some(first) = iter.next() { + range_to_css(first, dest)?; + for item in iter { + dest.write_str(", ")?; + range_to_css(item, dest)?; + } + Ok(()) + } else { + dest.write_str("auto") + } + } +} + +fn range_to_css(range: &Range>, dest: &mut W) -> fmt::Result +where W: fmt::Write { + bound_to_css(range.start, dest)?; + dest.write_char(' ')?; + bound_to_css(range.end, dest) +} + +fn bound_to_css(range: Option, dest: &mut W) -> fmt::Result where W: fmt::Write { + if let Some(finite) = range { + write!(dest, "{}", finite) + } else { + dest.write_str("infinite") + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 068696b1cdf..83429275e03 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -187,3 +187,26 @@ impl ToNsCssValue for counter_style::Symbol { } } } + +impl ToNsCssValue for counter_style::Ranges { + fn convert(&self, _nscssvalue: &mut nsCSSValue) { + if self.0.is_empty() { + //nscssvalue.set_auto(); // FIXME: add bindings for nsCSSValue::SetAutoValue + } else { + for range in &self.0 { + fn set_bound(bound: Option, nscssvalue: &mut nsCSSValue) { + if let Some(finite) = bound { + nscssvalue.set_integer(finite) + } else { + nscssvalue.set_enum(structs::NS_STYLE_COUNTER_RANGE_INFINITE as i32) + } + } + let mut start = nsCSSValue::null(); + let mut end = nsCSSValue::null(); + set_bound(range.start, &mut start); + set_bound(range.end, &mut end); + // FIXME: add bindings for nsCSSValuePairList + } + } + } +} From e27de3842d052470b88ca99e528ec69d958e7826 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 09:56:34 +0200 Subject: [PATCH 11/26] Add 'pad' descritor of @counter-style --- components/style/counter_style.rs | 27 +++++++++++++++++++++++++++ components/style/gecko/rules.rs | 11 +++++++++++ 2 files changed, 38 insertions(+) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index ad8897e99c2..62d9156467d 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -145,6 +145,9 @@ counter_style_descriptors! { /// https://drafts.csswg.org/css-counter-styles/#counter-style-range "range" range / eCSSCounterDesc_Range: Ranges = Ranges(Vec::new()); // Empty Vec represents 'auto' + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-pad + "pad" pad / eCSSCounterDesc_Pad: Pad = Pad(0, Symbol::String("".to_owned())); } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system @@ -331,3 +334,27 @@ fn bound_to_css(range: Option, dest: &mut W) -> fmt::Result where W: fmt dest.write_str("infinite") } } + +/// https://drafts.csswg.org/css-counter-styles/#counter-style-pad +#[derive(Debug)] +pub struct Pad(pub u32, pub Symbol); + +impl Parse for Pad { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + let pad_with = input.try(|input| Symbol::parse(context, input)); + let min_length = input.expect_integer()?; + if min_length < 0 { + return Err(()) + } + let pad_with = pad_with.or_else(|()| Symbol::parse(context, input))?; + Ok(Pad(min_length as u32, pad_with)) + } +} + +impl ToCss for Pad { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + write!(dest, "{}", self.0)?; + dest.write_char(' ')?; + self.1.to_css(dest) + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 83429275e03..6a1f3b6b5e4 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -210,3 +210,14 @@ impl ToNsCssValue for counter_style::Ranges { } } } + +impl ToNsCssValue for counter_style::Pad { + fn convert(&self, _nscssvalue: &mut nsCSSValue) { + let mut min_length = nsCSSValue::null(); + let mut pad_with = nsCSSValue::null(); + min_length.set_integer(self.0 as i32); + pad_with.set_from(&self.1); + // FIXME: add bindings for nsCSSValue::SetPairValue + //nscssvalue.set_pair(min_length, pad_with); + } +} From 0ba5cae7078ec3db21eadd523fabf5ec65d918ac Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 10:08:44 +0200 Subject: [PATCH 12/26] Add 'fallback' descriptor to @counter-style --- components/style/counter_style.rs | 21 +++++++++++++++++++ components/style/gecko/rules.rs | 6 ++++++ .../gecko_bindings/sugar/ns_css_value.rs | 5 +++++ 3 files changed, 32 insertions(+) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 62d9156467d..4d467190714 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -6,6 +6,7 @@ //! //! [counter-style]: https://drafts.csswg.org/css-counter-styles/ +use Atom; use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser, Token}; use cssparser::{serialize_string, serialize_identifier}; #[cfg(feature = "gecko")] use gecko::rules::CounterStyleDescriptors; @@ -148,6 +149,10 @@ counter_style_descriptors! { /// https://drafts.csswg.org/css-counter-styles/#counter-style-pad "pad" pad / eCSSCounterDesc_Pad: Pad = Pad(0, Symbol::String("".to_owned())); + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-fallback + "fallback" fallback / eCSSCounterDesc_Fallback: Fallback = + Fallback(CustomIdent(Atom::from("decimal"))); } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system @@ -358,3 +363,19 @@ impl ToCss for Pad { self.1.to_css(dest) } } + +/// https://drafts.csswg.org/css-counter-styles/#counter-style-fallback +#[derive(Debug)] +pub struct Fallback(pub CustomIdent); + +impl Parse for Fallback { + fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + parse_counter_style_name(input).map(Fallback) + } +} + +impl ToCss for Fallback { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + self.0.to_css(dest) + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 6a1f3b6b5e4..33fbe13cae7 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -221,3 +221,9 @@ impl ToNsCssValue for counter_style::Pad { //nscssvalue.set_pair(min_length, pad_with); } } + +impl ToNsCssValue for counter_style::Fallback { + fn convert(&self, nscssvalue: &mut nsCSSValue) { + nscssvalue.set_ident_from_atom(&self.0 .0) + } +} diff --git a/components/style/gecko_bindings/sugar/ns_css_value.rs b/components/style/gecko_bindings/sugar/ns_css_value.rs index d22f1148549..8d89db94c9c 100644 --- a/components/style/gecko_bindings/sugar/ns_css_value.rs +++ b/components/style/gecko_bindings/sugar/ns_css_value.rs @@ -124,6 +124,11 @@ impl nsCSSValue { self.set_string_from_atom_internal(s, nsCSSUnit::eCSSUnit_String) } + /// Set to a ident value from the given atom + pub fn set_ident_from_atom(&mut self, s: &Atom) { + self.set_string_from_atom_internal(s, nsCSSUnit::eCSSUnit_Ident) + } + /// Set to an identifier value pub fn set_ident(&mut self, s: &str) { self.set_string_internal(s, nsCSSUnit::eCSSUnit_Ident) From fe15663423fe5d090f937c1cf106b04c40f7d3fc Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 10:35:13 +0200 Subject: [PATCH 13/26] Add the 'symbols' descriptor for @counter-style --- components/style/counter_style.rs | 39 ++++++++++++++++++++++++++++++- components/style/gecko/rules.rs | 8 +++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 4d467190714..95e3d971f53 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -16,7 +16,7 @@ use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard}; use std::ascii::AsciiExt; use std::fmt; use std::ops::Range; -use style_traits::ToCss; +use style_traits::{ToCss, OneOrMoreCommaSeparated}; use values::CustomIdent; /// Parse the prelude of an @counter-style rule @@ -153,6 +153,9 @@ counter_style_descriptors! { /// https://drafts.csswg.org/css-counter-styles/#counter-style-fallback "fallback" fallback / eCSSCounterDesc_Fallback: Fallback = Fallback(CustomIdent(Atom::from("decimal"))); + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-symbols + "symbols" symbols / eCSSCounterDesc_Symbols: Symbols = Symbols(Vec::new()); } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system @@ -379,3 +382,37 @@ impl ToCss for Fallback { self.0.to_css(dest) } } + +/// https://drafts.csswg.org/css-counter-styles/#counter-style-symbols +#[derive(Debug)] +pub struct Symbols(pub Vec); + +impl Parse for Symbols { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + let mut symbols = Vec::new(); + loop { + if let Ok(s) = input.try(|input| Symbol::parse(context, input)) { + symbols.push(s) + } else { + if symbols.is_empty() { + return Err(()) + } else { + return Ok(Symbols(symbols)) + } + } + } + } +} + +impl ToCss for Symbols { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + let mut iter = self.0.iter(); + let first = iter.next().expect("expected at least one symbol"); + first.to_css(dest)?; + for item in iter { + dest.write_char(' ')?; + item.to_css(dest)?; + } + Ok(()) + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 33fbe13cae7..13681b4b8f9 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -227,3 +227,11 @@ impl ToNsCssValue for counter_style::Fallback { nscssvalue.set_ident_from_atom(&self.0 .0) } } + +impl ToNsCssValue for counter_style::Symbols { + fn convert(&self, _nscssvalue: &mut nsCSSValue) { + if !self.0.is_empty() { + // FIXME: add bindings for nsCSSValueList + } + } +} From 617e8e97689e8646ba48c1b5b620c9ae26567044 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 10:41:05 +0200 Subject: [PATCH 14/26] CSSOM requires @counter-style to keep track of which descriptors were specified --- components/style/counter_style.rs | 48 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 95e3d971f53..763037884b1 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -14,6 +14,7 @@ use cssparser::{serialize_string, serialize_identifier}; use parser::{ParserContext, log_css_error, Parse}; use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard}; use std::ascii::AsciiExt; +use std::borrow::Cow; use std::fmt; use std::ops::Range; use style_traits::{ToCss, OneOrMoreCommaSeparated}; @@ -27,7 +28,7 @@ pub fn parse_counter_style_name(input: &mut Parser) -> Result { /// Parse the body (inside `{}`) of an @counter-style rule pub fn parse_counter_style_body(name: CustomIdent, context: &ParserContext, input: &mut Parser) -> Result { - let mut rule = CounterStyleRule::initial(name); + let mut rule = CounterStyleRule::empty(name); { let parser = CounterStyleRuleParser { context: context, @@ -68,25 +69,38 @@ macro_rules! counter_style_descriptors { name: CustomIdent, $( #[$doc] - $ident: $ty, + $ident: Option<$ty>, )+ } impl CounterStyleRule { - fn initial(name: CustomIdent) -> Self { + fn empty(name: CustomIdent) -> Self { CounterStyleRule { name: name, $( - $ident: $initial, + $ident: None, )+ } } + $( + #[$doc] + pub fn $ident(&self) -> Cow<$ty> { + if let Some(ref value) = self.$ident { + Cow::Borrowed(value) + } else { + Cow::Owned($initial) + } + } + )+ + /// Convert to Gecko types #[cfg(feature = "gecko")] pub fn set_descriptors(&self, descriptors: &mut CounterStyleDescriptors) { $( - descriptors[nsCSSCounterDesc::$gecko_ident as usize].set_from(&self.$ident); + if let Some(ref value) = self.$ident { + descriptors[nsCSSCounterDesc::$gecko_ident as usize].set_from(value) + } )* } } @@ -103,7 +117,7 @@ macro_rules! counter_style_descriptors { // but in this case we do because we set the value as a side effect // rather than returning it. let value = input.parse_entirely(|i| Parse::parse(self.context, i))?; - self.rule.$ident = value + self.rule.$ident = Some(value) } )* _ => return Err(()) @@ -119,9 +133,11 @@ macro_rules! counter_style_descriptors { self.name.to_css(dest)?; dest.write_str(" {\n")?; $( - dest.write_str(concat!(" ", $name, ": "))?; - ToCss::to_css(&self.$ident, dest)?; - dest.write_str(";\n")?; + if let Some(ref value) = self.$ident { + dest.write_str(concat!(" ", $name, ": "))?; + ToCss::to_css(value, dest)?; + dest.write_str(";\n")?; + } )+ dest.write_str("}") } @@ -159,7 +175,7 @@ counter_style_descriptors! { } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum System { /// 'cyclic' Cyclic, @@ -225,7 +241,7 @@ impl ToCss for System { } /// https://drafts.csswg.org/css-counter-styles/#typedef-symbol -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum Symbol { /// String(String), @@ -256,7 +272,7 @@ impl ToCss for Symbol { } /// https://drafts.csswg.org/css-counter-styles/#counter-style-negative -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Negative(pub Symbol, pub Option); impl Parse for Negative { @@ -282,7 +298,7 @@ impl ToCss for Negative { /// https://drafts.csswg.org/css-counter-styles/#counter-style-range /// /// Empty Vec represents 'auto' -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Ranges(pub Vec>>); impl Parse for Ranges { @@ -344,7 +360,7 @@ fn bound_to_css(range: Option, dest: &mut W) -> fmt::Result where W: fmt } /// https://drafts.csswg.org/css-counter-styles/#counter-style-pad -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Pad(pub u32, pub Symbol); impl Parse for Pad { @@ -368,7 +384,7 @@ impl ToCss for Pad { } /// https://drafts.csswg.org/css-counter-styles/#counter-style-fallback -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Fallback(pub CustomIdent); impl Parse for Fallback { @@ -384,7 +400,7 @@ impl ToCss for Fallback { } /// https://drafts.csswg.org/css-counter-styles/#counter-style-symbols -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Symbols(pub Vec); impl Parse for Symbols { From f93a9a4b107f715b7d8f2f5eefad1bab5ed2e03c Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 10:52:47 +0200 Subject: [PATCH 15/26] =?UTF-8?q?Don=E2=80=99t=20make=20up=20initial=20val?= =?UTF-8?q?ues=20not=20in=20spec=20for=20@counter-style=20descriptors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- components/style/counter_style.rs | 63 +++++++++++++++++++++---------- components/style/gecko/rules.rs | 5 +-- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 763037884b1..0a3581a99b9 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -58,10 +58,29 @@ impl<'a, 'b> AtRuleParser for CounterStyleRuleParser<'a, 'b> { type AtRule = (); } +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: expr; )+ + $( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty = $initial: tt )+ ) => { /// An @counter-style rule #[derive(Debug)] @@ -84,14 +103,7 @@ macro_rules! counter_style_descriptors { } $( - #[$doc] - pub fn $ident(&self) -> Cow<$ty> { - if let Some(ref value) = self.$ident { - Cow::Borrowed(value) - } else { - Cow::Owned($initial) - } - } + accessor!(#[$doc] $name $ident: $ty = $initial); )+ /// Convert to Gecko types @@ -147,31 +159,42 @@ macro_rules! counter_style_descriptors { 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 = { + System::Symbolic + } /// 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 = { + Negative(Symbol::String("-".to_owned()), None) + } /// https://drafts.csswg.org/css-counter-styles/#counter-style-prefix - "prefix" prefix / eCSSCounterDesc_Prefix: Symbol = Symbol::String("".to_owned()); + "prefix" prefix / eCSSCounterDesc_Prefix: Symbol = { + Symbol::String("".to_owned()) + } /// https://drafts.csswg.org/css-counter-styles/#counter-style-suffix - "suffix" suffix / eCSSCounterDesc_Suffix: Symbol = Symbol::String(". ".to_owned()); + "suffix" suffix / eCSSCounterDesc_Suffix: Symbol = { + Symbol::String(". ".to_owned()) + } /// 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 = { + Ranges(Vec::new()) // Empty Vec represents 'auto' + } /// https://drafts.csswg.org/css-counter-styles/#counter-style-pad - "pad" pad / eCSSCounterDesc_Pad: Pad = Pad(0, Symbol::String("".to_owned())); + "pad" pad / eCSSCounterDesc_Pad: Pad = { + Pad(0, Symbol::String("".to_owned())) + } /// https://drafts.csswg.org/css-counter-styles/#counter-style-fallback - "fallback" fallback / eCSSCounterDesc_Fallback: Fallback = - Fallback(CustomIdent(Atom::from("decimal"))); + "fallback" fallback / eCSSCounterDesc_Fallback: Fallback = { + Fallback(CustomIdent(Atom::from("decimal"))) + } /// https://drafts.csswg.org/css-counter-styles/#counter-style-symbols - "symbols" symbols / eCSSCounterDesc_Symbols: Symbols = Symbols(Vec::new()); + "symbols" symbols / eCSSCounterDesc_Symbols: Symbols = ! } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 13681b4b8f9..e36b1a5c724 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -230,8 +230,7 @@ impl ToNsCssValue for counter_style::Fallback { impl ToNsCssValue for counter_style::Symbols { fn convert(&self, _nscssvalue: &mut nsCSSValue) { - if !self.0.is_empty() { - // FIXME: add bindings for nsCSSValueList - } + debug_assert!(!self.0.is_empty()); + // FIXME: add bindings for nsCSSValueList } } From 62d261add40ab6a1d38f48b32de6cf854d7bd459 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 11:06:41 +0200 Subject: [PATCH 16/26] Add 'additive-symbols' descriptor for @counter-style --- components/style/counter_style.rs | 38 +++++++++++++++++++++++++++---- components/style/gecko/rules.rs | 7 ++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 0a3581a99b9..88de60751f7 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -193,8 +193,11 @@ counter_style_descriptors! { Fallback(CustomIdent(Atom::from("decimal"))) } - /// https://drafts.csswg.org/css-counter-styles/#counter-style-symbols + /// https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-symbols "symbols" symbols / eCSSCounterDesc_Symbols: Symbols = ! + + /// https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-additive-symbols + "additive-symbols" additive_symbols / eCSSCounterDesc_AdditiveSymbols: Vec = ! } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system @@ -400,8 +403,7 @@ impl Parse for Pad { impl ToCss for Pad { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - write!(dest, "{}", self.0)?; - dest.write_char(' ')?; + write!(dest, "{} ", self.0)?; self.1.to_css(dest) } } @@ -422,7 +424,7 @@ impl ToCss for Fallback { } } -/// https://drafts.csswg.org/css-counter-styles/#counter-style-symbols +/// https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-symbols #[derive(Debug, Clone)] pub struct Symbols(pub Vec); @@ -455,3 +457,31 @@ impl ToCss for Symbols { Ok(()) } } + +/// https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-additive-symbols +#[derive(Debug, Clone)] +pub struct AdditiveSymbol { + value: i32, + symbol: Symbol, +} + +impl OneOrMoreCommaSeparated for AdditiveSymbol {} + +impl Parse for AdditiveSymbol { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + let value = input.try(|input| input.expect_integer()); + let symbol = Symbol::parse(context, input)?; + let value = value.or_else(|()| input.expect_integer())?; + Ok(AdditiveSymbol { + value: value, + symbol: symbol, + }) + } +} + +impl ToCss for AdditiveSymbol { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + write!(dest, "{} ", self.value)?; + self.symbol.to_css(dest) + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index e36b1a5c724..d9e27faf683 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -234,3 +234,10 @@ impl ToNsCssValue for counter_style::Symbols { // FIXME: add bindings for nsCSSValueList } } + +impl ToNsCssValue for Vec { + fn convert(&self, _nscssvalue: &mut nsCSSValue) { + debug_assert!(!self.is_empty()); + // FIXME: add bindings for nsCSSValuePairList + } +} From 6dc317f80bf0fb450255d0a48f00c5fae20137ed Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 11:24:25 +0200 Subject: [PATCH 17/26] Add speak-as descriptor for @counter-style --- components/style/counter_style.rs | 61 +++++++++++++++++++++++++++++++ components/style/gecko/rules.rs | 13 +++++++ 2 files changed, 74 insertions(+) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 88de60751f7..84bfb8806fb 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -198,6 +198,11 @@ counter_style_descriptors! { /// https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-additive-symbols "additive-symbols" additive_symbols / eCSSCounterDesc_AdditiveSymbols: Vec = ! + + /// https://drafts.csswg.org/css-counter-styles/#counter-style-speak-as + "speak-as" speak_as / eCSSCounterDesc_SpeakAs: SpeakAs = { + SpeakAs::Auto + } } /// https://drafts.csswg.org/css-counter-styles/#counter-style-system @@ -485,3 +490,59 @@ impl ToCss for AdditiveSymbol { self.symbol.to_css(dest) } } + +/// https://drafts.csswg.org/css-counter-styles/#counter-style-speak-as +#[derive(Debug, Clone)] +pub enum SpeakAs { + /// auto + Auto, + /// bullets + Bullets, + /// numbers + Numbers, + /// words + Words, + // /// spell-out, not supported, see bug 1024178 + // SpellOut, + /// + Other(CustomIdent), +} + +impl Parse for SpeakAs { + fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + let mut is_spell_out = false; + let result = input.try(|input| { + match_ignore_ascii_case! { &input.expect_ident()?, + "auto" => Ok(SpeakAs::Auto), + "bullets" => Ok(SpeakAs::Bullets), + "numbers" => Ok(SpeakAs::Numbers), + "words" => Ok(SpeakAs::Words), + "spell-out" => { + is_spell_out = true; + Err(()) + } + _ => Err(()) + } + }); + if is_spell_out { + // spell-out is not supported, but don’t parse it as a . + // See bug 1024178. + return Err(()) + } + result.or_else(|()| { + Ok(SpeakAs::Other(parse_counter_style_name(input)?)) + }) + } +} + +impl ToCss for SpeakAs { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + SpeakAs::Auto => dest.write_str("auto"), + SpeakAs::Bullets => dest.write_str("bullets"), + SpeakAs::Numbers => dest.write_str("numbers"), + SpeakAs::Words => dest.write_str("words"), + SpeakAs::Other(ref other) => other.to_css(dest), + } + } +} diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index d9e27faf683..d26fd08b689 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -241,3 +241,16 @@ impl ToNsCssValue for Vec { // FIXME: add bindings for nsCSSValuePairList } } + +impl ToNsCssValue for counter_style::SpeakAs { + fn convert(&self, nscssvalue: &mut nsCSSValue) { + use counter_style::SpeakAs::*; + match *self { + Auto => {} //nscssvalue.set_auto(), // FIXME: add bindings for nsCSSValue::SetAutoValue + Bullets => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SPEAKAS_BULLETS as i32), + Numbers => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SPEAKAS_NUMBERS as i32), + Words => nscssvalue.set_enum(structs::NS_STYLE_COUNTER_SPEAKAS_WORDS as i32), + Other(ref other) => nscssvalue.set_ident_from_atom(&other.0), + } + } +} From 331acfaf9bf9d6180a0134a9abda5f9e077cfa3c Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 11:44:06 +0200 Subject: [PATCH 18/26] Check rule-level @counter-style validity --- components/style/counter_style.rs | 39 ++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index 84bfb8806fb..ebeeaa9bc2d 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -28,6 +28,7 @@ pub fn parse_counter_style_name(input: &mut Parser) -> Result { /// Parse the body (inside `{}`) of an @counter-style rule pub fn parse_counter_style_body(name: CustomIdent, context: &ParserContext, input: &mut Parser) -> Result { + let start = input.position(); let mut rule = CounterStyleRule::empty(name); { let parser = CounterStyleRuleParser { @@ -44,7 +45,43 @@ pub fn parse_counter_style_body(name: CustomIdent, context: &ParserContext, inpu } } } - Ok(rule) + let error = match *rule.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(format!("Invalid @counter-style rule: 'system: {}' without 'symbols'", system)) + } + ref system @ System::Alphabetic | + ref system @ System::Numeric + if rule.symbols().unwrap().0.len() < 2 => { + let system = system.to_css_string(); + Some(format!("Invalid @counter-style rule: 'system: {}' less than two 'symbols'", + system)) + } + System::Additive if rule.additive_symbols.is_none() => { + let s = "Invalid @counter-style rule: 'system: additive' without 'additive-symbols'"; + Some(s.to_owned()) + } + System::Extends(_) if rule.symbols.is_some() => { + let s = "Invalid @counter-style rule: 'system: extends …' with 'symbols'"; + Some(s.to_owned()) + } + System::Extends(_) if rule.additive_symbols.is_some() => { + let s = "Invalid @counter-style rule: 'system: extends …' with 'additive-symbols'"; + Some(s.to_owned()) + } + _ => None + }; + if let Some(message) = error { + log_css_error(input, start, &message, context); + Err(()) + } else { + Ok(rule) + } } struct CounterStyleRuleParser<'a, 'b: 'a> { From ade56bbe8d65cee042eddd01d1c80fa92649069d Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 14 Apr 2017 12:14:33 +0200 Subject: [PATCH 19/26] Add missing 'additive-symbols' validity checks --- components/style/counter_style.rs | 46 +++++++++++++++++++++++-------- components/style/gecko/rules.rs | 4 +-- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/components/style/counter_style.rs b/components/style/counter_style.rs index ebeeaa9bc2d..772707cbfa8 100644 --- a/components/style/counter_style.rs +++ b/components/style/counter_style.rs @@ -234,7 +234,7 @@ counter_style_descriptors! { "symbols" symbols / eCSSCounterDesc_Symbols: Symbols = ! /// https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-additive-symbols - "additive-symbols" additive_symbols / eCSSCounterDesc_AdditiveSymbols: Vec = ! + "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 = { @@ -502,26 +502,50 @@ impl ToCss for Symbols { /// https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-additive-symbols #[derive(Debug, Clone)] -pub struct AdditiveSymbol { - value: i32, +pub struct AdditiveSymbols(pub Vec); + +impl Parse for AdditiveSymbols { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + let tuples = Vec::::parse(context, input)?; + // FIXME maybe? https://github.com/w3c/csswg-drafts/issues/1220 + if tuples.windows(2).any(|window| window[0].value <= window[1].value) { + return Err(()) + } + Ok(AdditiveSymbols(tuples)) + } +} + +impl ToCss for AdditiveSymbols { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + self.0.to_css(dest) + } +} + +/// && +#[derive(Debug, Clone)] +pub struct AdditiveTuple { + value: u32, symbol: Symbol, } -impl OneOrMoreCommaSeparated for AdditiveSymbol {} +impl OneOrMoreCommaSeparated for AdditiveTuple {} -impl Parse for AdditiveSymbol { +impl Parse for AdditiveTuple { fn parse(context: &ParserContext, input: &mut Parser) -> Result { - let value = input.try(|input| input.expect_integer()); - let symbol = Symbol::parse(context, input)?; - let value = value.or_else(|()| input.expect_integer())?; - Ok(AdditiveSymbol { - value: value, + let symbol = input.try(|input| Symbol::parse(context, input)); + let value = input.expect_integer()?; + if value < 0 { + return Err(()) + } + let symbol = symbol.or_else(|()| Symbol::parse(context, input))?; + Ok(AdditiveTuple { + value: value as u32, symbol: symbol, }) } } -impl ToCss for AdditiveSymbol { +impl ToCss for AdditiveTuple { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { write!(dest, "{} ", self.value)?; self.symbol.to_css(dest) diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index d26fd08b689..d2d78de238a 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -235,9 +235,9 @@ impl ToNsCssValue for counter_style::Symbols { } } -impl ToNsCssValue for Vec { +impl ToNsCssValue for counter_style::AdditiveSymbols { fn convert(&self, _nscssvalue: &mut nsCSSValue) { - debug_assert!(!self.is_empty()); + debug_assert!(!self.0.is_empty()); // FIXME: add bindings for nsCSSValuePairList } } From e7a2a98d8a1dd77232c15435e913194174ed18a3 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 24 Apr 2017 10:04:09 +0200 Subject: [PATCH 20/26] Make the style::counter_style module a directory --- components/style/{counter_style.rs => counter_style/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename components/style/{counter_style.rs => counter_style/mod.rs} (100%) diff --git a/components/style/counter_style.rs b/components/style/counter_style/mod.rs similarity index 100% rename from components/style/counter_style.rs rename to components/style/counter_style/mod.rs From 82c04113d09cf728542df2620c4397f55028df40 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 24 Apr 2017 10:49:13 +0200 Subject: [PATCH 21/26] Make predefined counter style names ASCII-lowercase. --- components/style/counter_style/mod.rs | 23 ++++++- components/style/counter_style/predefined.rs | 60 +++++++++++++++++++ .../style/counter_style/update_predefined.py | 35 +++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 components/style/counter_style/predefined.rs create mode 100755 components/style/counter_style/update_predefined.py diff --git a/components/style/counter_style/mod.rs b/components/style/counter_style/mod.rs index 772707cbfa8..f27aecc8549 100644 --- a/components/style/counter_style/mod.rs +++ b/components/style/counter_style/mod.rs @@ -22,7 +22,28 @@ use values::CustomIdent; /// Parse the prelude of an @counter-style rule pub fn parse_counter_style_name(input: &mut Parser) -> Result { - CustomIdent::from_ident(input.expect_ident()?, &["decimal", "none"]) + macro_rules! predefined { + ($($name: expr,)+) => { + { + ascii_case_insensitive_phf_map! { + // FIXME: use static atoms https://github.com/rust-lang/rust/issues/33156 + predefined -> &'static str = { + $( + $name => $name, + )+ + } + } + + let ident = input.expect_ident()?; + if let Some(&lower_cased) = predefined(&ident) { + Ok(CustomIdent(Atom::from(lower_cased))) + } else { + CustomIdent::from_ident(ident, &["decimal", "none"]) + } + } + } + } + include!("predefined.rs") } /// Parse the body (inside `{}`) of an @counter-style rule diff --git a/components/style/counter_style/predefined.rs b/components/style/counter_style/predefined.rs new file mode 100644 index 00000000000..dee17e90377 --- /dev/null +++ b/components/style/counter_style/predefined.rs @@ -0,0 +1,60 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +predefined! { + "decimal-leading-zero", + "arabic-indic", + "armenian", + "upper-armenian", + "lower-armenian", + "bengali", + "cambodian", + "khmer", + "cjk-decimal", + "devanagari", + "georgian", + "gujarati", + "gurmukhi", + "hebrew", + "kannada", + "lao", + "malayalam", + "mongolian", + "myanmar", + "oriya", + "persian", + "lower-roman", + "upper-roman", + "tamil", + "telugu", + "thai", + "tibetan", + "lower-alpha", + "lower-latin", + "upper-alpha", + "upper-latin", + "cjk-earthly-branch", + "cjk-heavenly-stem", + "lower-greek", + "hiragana", + "hiragana-iroha", + "katakana", + "katakana-iroha", + "disc", + "circle", + "square", + "disclosure-open", + "disclosure-closed", + "japanese-informal", + "japanese-formal", + "korean-hangul-formal", + "korean-hanja-informal", + "korean-hanja-formal", + "simp-chinese-informal", + "simp-chinese-formal", + "trad-chinese-informal", + "trad-chinese-formal", + "cjk-ideographic", + "ethiopic-numeric", +} diff --git a/components/style/counter_style/update_predefined.py b/components/style/counter_style/update_predefined.py new file mode 100755 index 00000000000..9e64545b593 --- /dev/null +++ b/components/style/counter_style/update_predefined.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python + +# 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 http://mozilla.org/MPL/2.0/. */ + +import os.path +import re +import urllib + + +def main(filename): + names = [ + re.search('>([^>]+)(| Date: Tue, 25 Apr 2017 09:14:32 +0200 Subject: [PATCH 22/26] Keep custom-ident and string separate in animation/keyframes name. --- components/script/dom/csskeyframesrule.rs | 14 +++--- components/style/animation.rs | 19 +++++--- components/style/gecko_string_cache/mod.rs | 6 +++ components/style/matching.rs | 2 +- components/style/properties/gecko.mako.rs | 16 +++++-- .../style/properties/longhand/box.mako.rs | 45 ++++++++--------- .../style/properties/properties.mako.rs | 2 +- components/style/stylesheets.rs | 16 +++---- components/style/stylist.rs | 4 +- components/style/values/mod.rs | 48 ++++++++++++++++++- tests/unit/style/lib.rs | 2 +- tests/unit/style/parsing/animation.rs | 12 ++--- tests/unit/style/stylesheets.rs | 4 +- 13 files changed, 123 insertions(+), 67 deletions(-) diff --git a/components/script/dom/csskeyframesrule.rs b/components/script/dom/csskeyframesrule.rs index 5a03dc41658..a3715e914ed 100644 --- a/components/script/dom/csskeyframesrule.rs +++ b/components/script/dom/csskeyframesrule.rs @@ -5,7 +5,7 @@ use cssparser::Parser; use dom::bindings::codegen::Bindings::CSSKeyframesRuleBinding; use dom::bindings::codegen::Bindings::CSSKeyframesRuleBinding::CSSKeyframesRuleMethods; -use dom::bindings::error::{Error, ErrorResult}; +use dom::bindings::error::ErrorResult; use dom::bindings::inheritance::Castable; use dom::bindings::js::{MutNullableJS, Root}; use dom::bindings::reflector::{DomObject, reflect_dom_object}; @@ -20,7 +20,7 @@ use std::sync::Arc; use style::keyframes::{Keyframe, KeyframeSelector}; use style::shared_lock::{Locked, ToCssWithGuard}; use style::stylesheets::KeyframesRule; -use style::values::CustomIdent; +use style::values::KeyframesName; #[dom_struct] pub struct CSSKeyframesRule { @@ -107,15 +107,15 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule { // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name fn Name(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - DOMString::from(&*self.keyframesrule.read_with(&guard).name.0) + DOMString::from(&**self.keyframesrule.read_with(&guard).name.as_atom()) } // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name fn SetName(&self, value: DOMString) -> ErrorResult { - // https://github.com/w3c/csswg-drafts/issues/801 - // Setting this property to a CSS-wide keyword or `none` will - // throw a Syntax Error. - let name = CustomIdent::from_ident(value.into(), &["none"]).map_err(|()| Error::Syntax)?; + // Spec deviation: https://github.com/w3c/csswg-drafts/issues/801 + // Setting this property to a CSS-wide keyword or `none` does not throw, + // it stores a value that serializes as a quoted string. + let name = KeyframesName::from_ident(value.into()); let mut guard = self.cssrule.shared_lock().write(); self.keyframesrule.write_with(&mut guard).name = name; Ok(()) diff --git a/components/style/animation.rs b/components/style/animation.rs index 604f9ee62bf..2229d485591 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -464,14 +464,20 @@ pub fn maybe_start_animations(context: &SharedStyleContext, let box_style = new_style.get_box(); for (i, name) in box_style.animation_name_iter().enumerate() { + let name = if let Some(atom) = name.as_atom() { + atom + } else { + continue + }; + debug!("maybe_start_animations: name={}", name); let total_duration = box_style.animation_duration_mod(i).seconds(); if total_duration == 0. { continue } - if let Some(ref anim) = context.stylist.animations().get(&name.0 .0) { - debug!("maybe_start_animations: animation {} found", name.0 .0); + if let Some(ref anim) = context.stylist.animations().get(name) { + debug!("maybe_start_animations: animation {} found", name); // If this animation doesn't have any keyframe, we can just continue // without submitting it to the compositor, since both the first and @@ -506,7 +512,7 @@ pub fn maybe_start_animations(context: &SharedStyleContext, new_animations_sender - .send(Animation::Keyframes(node, name.0 .0.clone(), KeyframesAnimationState { + .send(Animation::Keyframes(node, name.clone(), KeyframesAnimationState { started_at: animation_start, duration: duration as f64, delay: delay as f64, @@ -584,9 +590,10 @@ pub fn update_style_for_animation(context: &SharedStyleContext, debug_assert!(!animation.steps.is_empty()); - let maybe_index = style.get_box() - .animation_name_iter() - .position(|animation_name| *name == animation_name.0 .0); + let maybe_index = style + .get_box() + .animation_name_iter() + .position(|animation_name| Some(name) == animation_name.as_atom()); let index = match maybe_index { Some(index) => index, diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index 82589b645ea..b612c027c45 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -163,6 +163,12 @@ impl WeakAtom { } } + /// Returns whether this atom is the empty string. + #[inline] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Returns the atom as a mutable pointer. #[inline] pub fn as_ptr(&self) -> *mut nsIAtom { diff --git a/components/style/matching.rs b/components/style/matching.rs index cbaa6d12afb..201f6c50878 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -560,7 +560,7 @@ trait PrivateMatchMethods: TElement { pseudo: Option<&PseudoElement>) -> bool { let ref new_box_style = new_values.get_box(); let has_new_animation_style = new_box_style.animation_name_count() >= 1 && - new_box_style.animation_name_at(0).0 .0.len() != 0; + new_box_style.animation_name_at(0).0.is_some(); let has_animations = self.has_css_animations(pseudo); old_values.as_ref().map_or(has_new_animation_style, |ref old| { diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index b74bb9e8cd3..57ebd1114ff 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -62,7 +62,7 @@ use std::ptr; use std::sync::Arc; use std::cmp; use values::computed::ToComputedValue; -use values::{Either, Auto, CustomIdent}; +use values::{Either, Auto, KeyframesName}; use computed_values::border_style; pub mod style_structs { @@ -2202,16 +2202,22 @@ fn static_assert() { self.gecko.mAnimationNameCount = v.0.len() as u32; for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) { // TODO This is inefficient. We should fix this in bug 1329169. - gecko.mName.assign(servo.0 .0.as_slice()); + gecko.mName.assign(match servo.0 { + Some(ref name) => name.as_atom().as_slice(), + None => &[], // Empty string for 'none' + }); } } pub fn animation_name_at(&self, index: usize) -> longhands::animation_name::computed_value::SingleComputedValue { - use Atom; use properties::longhands::animation_name::single_value::SpecifiedValue as AnimationName; // XXX: Is there any effective ways? - AnimationName(CustomIdent(Atom::from( - String::from_utf16_lossy(&self.gecko.mAnimations[index].mName[..])))) + let atom = &self.gecko.mAnimations[index].mName; + if atom.is_empty() { + AnimationName(None) + } else { + AnimationName(Some(KeyframesName::from_ident(atom.to_string()))) + } } pub fn copy_animation_name_from(&mut self, other: &Self) { unsafe { self.gecko.mAnimations.ensure_len(other.gecko.mAnimations.len()) }; diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index e5aae336283..2806a04e803 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -796,7 +796,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", use std::ops::Deref; use style_traits::ToCss; use values::computed::ComputedValueAsSpecified; - use values::{HasViewportPercentage, CustomIdent}; + use values::{HasViewportPercentage, KeyframesName}; pub mod computed_value { pub use super::SpecifiedValue as T; @@ -804,7 +804,14 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", #[derive(Clone, Debug, Hash, Eq, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct SpecifiedValue(pub CustomIdent); + pub struct SpecifiedValue(pub Option); + + impl SpecifiedValue { + /// As an Atom + pub fn as_atom(&self) -> Option< &Atom> { + self.0.as_ref().map(|n| n.as_atom()) + } + } #[inline] pub fn get_initial_value() -> computed_value::T { @@ -813,44 +820,32 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", #[inline] pub fn get_initial_specified_value() -> SpecifiedValue { - SpecifiedValue(CustomIdent(atom!(""))) + SpecifiedValue(None) } impl fmt::Display for SpecifiedValue { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.0 .0.fmt(f) + self.to_css(f) } } impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if self.0 .0 == atom!("") { - dest.write_str("none") + if let Some(ref name) = self.0 { + name.to_css(dest) } else { - self.0.to_css(dest) + dest.write_str("none") } } } impl Parse for SpecifiedValue { - fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result { - use cssparser::Token; - use std::ascii::AsciiExt; - use values::CustomIdent; - - let atom = match input.next() { - Ok(Token::Ident(value)) => { - if value.eq_ignore_ascii_case("none") { - // FIXME We may want to support `@keyframes ""` at some point. - CustomIdent(atom!("")) - } else { - CustomIdent::from_ident(value, &[])? - } - } - Ok(Token::QuotedString(value)) => CustomIdent(Atom::from(value)), - _ => return Err(()), - }; - Ok(SpecifiedValue(atom)) + fn parse(context: &ParserContext, input: &mut ::cssparser::Parser) -> Result { + if let Ok(name) = input.try(|input| KeyframesName::parse(context, input)) { + Ok(SpecifiedValue(Some(name))) + } else { + input.expect_ident_matching("none").map(|()| SpecifiedValue(None)) + } } } no_viewport_percentage!(SpecifiedValue); diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 403399acfa9..69932549fe2 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1617,7 +1617,7 @@ pub mod style_structs { /// Returns whether there is any animation specified with /// animation-name other than `none`. pub fn specifies_animations(&self) -> bool { - self.animation_name_iter().any(|name| name.0 .0 != atom!("")) + self.animation_name_iter().any(|name| name.0.is_some()) } /// Returns whether there are any transitions specified. diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index bf7c10178f0..5460dba620b 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -6,10 +6,10 @@ #![deny(missing_docs)] -use {Atom, Prefix, Namespace}; +use {Prefix, Namespace}; use counter_style::{CounterStyleRule, parse_counter_style_name, parse_counter_style_body}; use cssparser::{AtRuleParser, Parser, QualifiedRuleParser}; -use cssparser::{AtRuleType, RuleListParser, Token, parse_one_rule}; +use cssparser::{AtRuleType, RuleListParser, SourcePosition, parse_one_rule}; use cssparser::ToCss as ParserToCss; use error_reporting::{ParseErrorReporter, NullReporter}; #[cfg(feature = "servo")] @@ -41,7 +41,7 @@ use str::starts_with_ignore_ascii_case; use style_traits::ToCss; use stylist::FnvHashMap; use supports::SupportsCondition; -use values::CustomIdent; +use values::{CustomIdent, KeyframesName}; use values::specified::url::SpecifiedUrl; use viewport::ViewportRule; @@ -519,7 +519,7 @@ impl ToCssWithGuard for ImportRule { #[derive(Debug)] pub struct KeyframesRule { /// The name of the current animation. - pub name: CustomIdent, + pub name: KeyframesName, /// The keyframes specified for this CSS rule. pub keyframes: Vec>>, /// Vendor prefix type the @keyframes has. @@ -931,7 +931,7 @@ enum AtRulePrelude { /// A @viewport rule prelude. Viewport, /// A @keyframes rule, with its animation name and vendor prefix if exists. - Keyframes(CustomIdent, Option), + Keyframes(KeyframesName, Option), /// A @page rule prelude. Page, } @@ -1139,11 +1139,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { // Servo should not support @-moz-keyframes. return Err(()) } - let name = match input.next() { - Ok(Token::Ident(value)) => CustomIdent::from_ident(value, &["none"])?, - Ok(Token::QuotedString(value)) => CustomIdent(Atom::from(value)), - _ => return Err(()) - }; + let name = KeyframesName::parse(self.context, input)?; Ok(AtRuleType::WithBlock(AtRulePrelude::Keyframes(name, prefix))) }, diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 4f5489f0d8c..8ae9998c4cb 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -349,13 +349,13 @@ impl Stylist { // Don't let a prefixed keyframes animation override a non-prefixed one. let needs_insertion = keyframes_rule.vendor_prefix.is_none() || - self.animations.get(&keyframes_rule.name.0).map_or(true, |rule| + self.animations.get(keyframes_rule.name.as_atom()).map_or(true, |rule| rule.vendor_prefix.is_some()); if needs_insertion { let animation = KeyframesAnimation::from_keyframes( &keyframes_rule.keyframes, keyframes_rule.vendor_prefix.clone(), guard); debug!("Found valid keyframe animation: {:?}", animation); - self.animations.insert(keyframes_rule.name.0.clone(), animation); + self.animations.insert(keyframes_rule.name.as_atom().clone(), animation); } } CssRule::FontFace(ref rule) => { diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index 7379cbad846..5caf64fc0ea 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -9,7 +9,7 @@ #![deny(missing_docs)] use Atom; -pub use cssparser::{RGBA, Parser, serialize_identifier}; +pub use cssparser::{RGBA, Token, Parser, serialize_identifier, serialize_string}; use parser::{Parse, ParserContext}; use std::ascii::AsciiExt; use std::borrow::Cow; @@ -240,6 +240,52 @@ impl ToCss for CustomIdent { } } +/// https://drafts.csswg.org/css-animations/#typedef-keyframes-name +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub enum KeyframesName { + /// + Ident(CustomIdent), + /// + QuotedString(Atom), +} + +impl KeyframesName { + /// https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name + pub fn from_ident(value: String) -> Self { + match CustomIdent::from_ident((&*value).into(), &["none"]) { + Ok(ident) => KeyframesName::Ident(ident), + Err(()) => KeyframesName::QuotedString(value.into()), + } + } + + /// The name as an Atom + pub fn as_atom(&self) -> &Atom { + match *self { + KeyframesName::Ident(ref ident) => &ident.0, + KeyframesName::QuotedString(ref atom) => atom, + } + } +} + +impl Parse for KeyframesName { + fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + match input.next() { + Ok(Token::Ident(s)) => Ok(KeyframesName::Ident(CustomIdent::from_ident(s, &["none"])?)), + Ok(Token::QuotedString(s)) => Ok(KeyframesName::QuotedString(s.into())), + _ => Err(()) + } + } +} + +impl ToCss for KeyframesName { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + KeyframesName::Ident(ref ident) => ident.to_css(dest), + KeyframesName::QuotedString(ref atom) => serialize_string(&atom.to_string(), dest), + } + } +} // A type for possible values for min- and max- flavors of width, // height, block-size, and inline-size. diff --git a/tests/unit/style/lib.rs b/tests/unit/style/lib.rs index f6a39d1a656..baddd4da618 100644 --- a/tests/unit/style/lib.rs +++ b/tests/unit/style/lib.rs @@ -13,7 +13,7 @@ extern crate parking_lot; extern crate rayon; extern crate rustc_serialize; extern crate selectors; -#[macro_use] extern crate servo_atoms; +extern crate servo_atoms; extern crate servo_config; extern crate servo_url; extern crate style; diff --git a/tests/unit/style/parsing/animation.rs b/tests/unit/style/parsing/animation.rs index 936d6c644f7..37cb3c5037b 100644 --- a/tests/unit/style/parsing/animation.rs +++ b/tests/unit/style/parsing/animation.rs @@ -7,7 +7,7 @@ use servo_atoms::Atom; use style::parser::Parse; use style::properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount; use style::properties::longhands::animation_name; -use style::values::CustomIdent; +use style::values::{KeyframesName, CustomIdent}; use style_traits::ToCss; #[test] @@ -15,13 +15,13 @@ fn test_animation_name() { use self::animation_name::single_value::SpecifiedValue as SingleValue; let other_name = Atom::from("other-name"); assert_eq!(parse_longhand!(animation_name, "none"), - animation_name::SpecifiedValue(vec![SingleValue(CustomIdent(atom!("")))])); + animation_name::SpecifiedValue(vec![SingleValue(None)])); assert_eq!(parse_longhand!(animation_name, "other-name, none, 'other-name', \"other-name\""), animation_name::SpecifiedValue( - vec![SingleValue(CustomIdent(other_name.clone())), - SingleValue(CustomIdent(atom!(""))), - SingleValue(CustomIdent(other_name.clone())), - SingleValue(CustomIdent(other_name.clone()))])); + vec![SingleValue(Some(KeyframesName::Ident(CustomIdent(other_name.clone())))), + SingleValue(None), + SingleValue(Some(KeyframesName::QuotedString(other_name.clone()))), + SingleValue(Some(KeyframesName::QuotedString(other_name.clone())))])); } #[test] diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 9a56ee80d53..dd1762ee686 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -23,7 +23,7 @@ use style::properties::longhands::animation_play_state; use style::shared_lock::SharedRwLock; use style::stylesheets::{Origin, Namespaces}; use style::stylesheets::{Stylesheet, NamespaceRule, CssRule, CssRules, StyleRule, KeyframesRule}; -use style::values::CustomIdent; +use style::values::{KeyframesName, CustomIdent}; use style::values::specified::{LengthOrPercentageOrAuto, Percentage}; pub fn block_from(iterable: I) -> PropertyDeclarationBlock @@ -222,7 +222,7 @@ fn test_parse_stylesheet() { ]))), }))), CssRule::Keyframes(Arc::new(stylesheet.shared_lock.wrap(KeyframesRule { - name: CustomIdent("foo".into()), + name: KeyframesName::Ident(CustomIdent("foo".into())), keyframes: vec![ Arc::new(stylesheet.shared_lock.wrap(Keyframe { selector: KeyframeSelector::new_for_unit_testing( From 0ff64bdc59e75c4fa16927db5cc8797bf60f9a36 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 25 Apr 2017 10:44:10 +0200 Subject: [PATCH 23/26] Allow 'decimal' and 'none' in `` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … other than in `@counter-style`. --- components/atoms/build.rs | 15 +++++++++++++- components/atoms/static_atoms.txt | 2 ++ components/style/counter_style/mod.rs | 3 ++- components/style/counter_style/predefined.rs | 1 + .../style/counter_style/update_predefined.py | 3 --- components/style/gecko_string_cache/mod.rs | 20 +++++++++++++++++++ components/style/stylesheets.rs | 6 ++++++ 7 files changed, 45 insertions(+), 5 deletions(-) diff --git a/components/atoms/build.rs b/components/atoms/build.rs index 4de629d288c..c8edc9bcd5a 100644 --- a/components/atoms/build.rs +++ b/components/atoms/build.rs @@ -12,7 +12,20 @@ use std::path::Path; fn main() { let static_atoms = Path::new(&env::var("CARGO_MANIFEST_DIR").unwrap()).join("static_atoms.txt"); let static_atoms = BufReader::new(File::open(&static_atoms).unwrap()); - string_cache_codegen::AtomType::new("Atom", "atom!") + let mut atom_type = string_cache_codegen::AtomType::new("Atom", "atom!"); + + macro_rules! predefined { + ($($name: expr,)+) => { + { + $( + atom_type.atom($name); + )+ + } + } + } + include!("../style/counter_style/predefined.rs"); + + atom_type .atoms(static_atoms.lines().map(Result::unwrap)) .write_to_file(&Path::new(&env::var("OUT_DIR").unwrap()).join("atom.rs")) .unwrap(); diff --git a/components/atoms/static_atoms.txt b/components/atoms/static_atoms.txt index df9cd426984..a41aae29289 100644 --- a/components/atoms/static_atoms.txt +++ b/components/atoms/static_atoms.txt @@ -8,6 +8,8 @@ left center right +none + hidden submit button diff --git a/components/style/counter_style/mod.rs b/components/style/counter_style/mod.rs index f27aecc8549..8397ebff755 100644 --- a/components/style/counter_style/mod.rs +++ b/components/style/counter_style/mod.rs @@ -38,7 +38,7 @@ pub fn parse_counter_style_name(input: &mut Parser) -> Result { if let Some(&lower_cased) = predefined(&ident) { Ok(CustomIdent(Atom::from(lower_cased))) } else { - CustomIdent::from_ident(ident, &["decimal", "none"]) + CustomIdent::from_ident(ident, &[]) } } } @@ -248,6 +248,7 @@ counter_style_descriptors! { /// 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"))) } diff --git a/components/style/counter_style/predefined.rs b/components/style/counter_style/predefined.rs index dee17e90377..7b27514cd33 100644 --- a/components/style/counter_style/predefined.rs +++ b/components/style/counter_style/predefined.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ predefined! { + "decimal", "decimal-leading-zero", "arabic-indic", "armenian", diff --git a/components/style/counter_style/update_predefined.py b/components/style/counter_style/update_predefined.py index 9e64545b593..3e5be33e448 100755 --- a/components/style/counter_style/update_predefined.py +++ b/components/style/counter_style/update_predefined.py @@ -25,9 +25,6 @@ def main(filename): predefined! { """) for name in names: - # FIXME https://github.com/w3c/csswg-drafts/issues/1285 - if name == 'decimal': - continue f.write(' "%s",\n' % name) f.write('}\n') diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index b612c027c45..63ba6d7977c 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -13,6 +13,7 @@ use gecko_bindings::bindings::Gecko_ReleaseAtom; use gecko_bindings::structs::nsIAtom; use nsstring::nsAString; use precomputed_hash::PrecomputedHash; +use std::ascii::AsciiExt; use std::borrow::{Cow, Borrow}; use std::char::{self, DecodeUtf16}; use std::fmt::{self, Write}; @@ -224,6 +225,25 @@ impl Atom { Atom(WeakAtom::new(ptr)) } } + + /// Return whether two atoms are ASCII-case-insensitive matches + pub fn eq_ignore_ascii_case(&self, other: &Self) -> bool { + let a = self.as_slice(); + let b = other.as_slice(); + a.len() == b.len() && a.iter().zip(b).all(|(&a16, &b16)| { + if a16 <= 0x7F && b16 <= 0x7F { + (a16 as u8).eq_ignore_ascii_case(&(b16 as u8)) + } else { + a16 == b16 + } + }) + } + + /// Return whether this atom is an ASCII-case-insensitive match for the given string + pub fn eq_str_ignore_ascii_case(&self, other: &str) -> bool { + self.chars().map(|r| r.map(|c: char| c.to_ascii_lowercase())) + .eq(other.chars().map(|c: char| Ok(c.to_ascii_lowercase()))) + } } impl Hash for Atom { diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 5460dba620b..4012761a6ef 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -1117,6 +1117,12 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { return Err(()) } let name = parse_counter_style_name(input)?; + // FIXME: use static atoms and eq_ignore_ascii_case + // https://bugzilla.mozilla.org/show_bug.cgi?id=1359323 + // "decimal" is already lower-cased by `parse_counter_style_name`. + if name.0 == "decimal" || name.0.eq_str_ignore_ascii_case("none") { + return Err(()) + } Ok(AtRuleType::WithBlock(AtRulePrelude::CounterStyle(name))) }, "viewport" => { From be38c9a5a754a16d417b6885a57d15c5bc4cc414 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 26 Apr 2017 05:15:20 +0200 Subject: [PATCH 24/26] Consider `foo` and `"foo"` equal as keyframes/animation names. --- components/style/values/mod.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index 5caf64fc0ea..430eac27419 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -14,6 +14,7 @@ use parser::{Parse, ParserContext}; use std::ascii::AsciiExt; use std::borrow::Cow; use std::fmt::{self, Debug}; +use std::hash; use style_traits::ToCss; macro_rules! define_numbered_css_keyword_enum { @@ -241,7 +242,7 @@ impl ToCss for CustomIdent { } /// https://drafts.csswg.org/css-animations/#typedef-keyframes-name -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum KeyframesName { /// @@ -268,6 +269,20 @@ impl KeyframesName { } } +impl Eq for KeyframesName {} + +impl PartialEq for KeyframesName { + fn eq(&self, other: &Self) -> bool { + self.as_atom() == other.as_atom() + } +} + +impl hash::Hash for KeyframesName { + fn hash(&self, state: &mut H) where H: hash::Hasher { + self.as_atom().hash(state) + } +} + impl Parse for KeyframesName { fn parse(_context: &ParserContext, input: &mut Parser) -> Result { match input.next() { From 029150c770d887acabb3667b934efc53c6e1629b Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 26 Apr 2017 05:39:36 +0200 Subject: [PATCH 25/26] Exclude 'none' from after all. https://github.com/w3c/csswg-drafts/issues/1295 --- components/style/counter_style/mod.rs | 3 ++- components/style/stylesheets.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/components/style/counter_style/mod.rs b/components/style/counter_style/mod.rs index 8397ebff755..ca27a650334 100644 --- a/components/style/counter_style/mod.rs +++ b/components/style/counter_style/mod.rs @@ -38,7 +38,8 @@ pub fn parse_counter_style_name(input: &mut Parser) -> Result { if let Some(&lower_cased) = predefined(&ident) { Ok(CustomIdent(Atom::from(lower_cased))) } else { - CustomIdent::from_ident(ident, &[]) + // https://github.com/w3c/csswg-drafts/issues/1295 excludes "none" + CustomIdent::from_ident(ident, &["none"]) } } } diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 4012761a6ef..9986328cb00 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -6,10 +6,10 @@ #![deny(missing_docs)] -use {Prefix, Namespace}; +use {Atom, Prefix, Namespace}; use counter_style::{CounterStyleRule, parse_counter_style_name, parse_counter_style_body}; use cssparser::{AtRuleParser, Parser, QualifiedRuleParser}; -use cssparser::{AtRuleType, RuleListParser, SourcePosition, parse_one_rule}; +use cssparser::{AtRuleType, RuleListParser, parse_one_rule}; use cssparser::ToCss as ParserToCss; use error_reporting::{ParseErrorReporter, NullReporter}; #[cfg(feature = "servo")] @@ -1117,10 +1117,10 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { return Err(()) } let name = parse_counter_style_name(input)?; - // FIXME: use static atoms and eq_ignore_ascii_case - // https://bugzilla.mozilla.org/show_bug.cgi?id=1359323 - // "decimal" is already lower-cased by `parse_counter_style_name`. - if name.0 == "decimal" || name.0.eq_str_ignore_ascii_case("none") { + // ASCII-case-insensitive matches for "decimal" are already lower-cased + // by `parse_counter_style_name`, so we can use == here. + // FIXME: https://bugzilla.mozilla.org/show_bug.cgi?id=1359323 use atom!("decimal") + if name.0 == Atom::from("decimal") { return Err(()) } Ok(AtRuleType::WithBlock(AtRulePrelude::CounterStyle(name))) From 1b419007d17ef8865b18f685f6a9e2ae3643c6fd Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 26 Apr 2017 07:37:10 +0200 Subject: [PATCH 26/26] =?UTF-8?q?CSSKeyframesRule::name=20setter=20doesn?= =?UTF-8?q?=E2=80=99t=20throw=20anymore?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/w3c/csswg-drafts/commit/063dc05e478096e90d377909414b6a1b051714ae https://github.com/w3c/web-platform-tests/pull/5695 --- tests/wpt/metadata/MANIFEST.json | 2 +- .../cssom/CSSKeyframesRule.html | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index 8c0c34ff0a8..fc43eb22216 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -553680,7 +553680,7 @@ "testharness" ], "cssom/CSSKeyframesRule.html": [ - "3efb8e5cef257a0b433192742d526709357b24c7", + "bca997a63c1389ef6d14aac2f32ab770fbd15ec4", "testharness" ], "cssom/CSSNamespaceRule.html": [ diff --git a/tests/wpt/web-platform-tests/cssom/CSSKeyframesRule.html b/tests/wpt/web-platform-tests/cssom/CSSKeyframesRule.html index 9cf387def86..68453498767 100644 --- a/tests/wpt/web-platform-tests/cssom/CSSKeyframesRule.html +++ b/tests/wpt/web-platform-tests/cssom/CSSKeyframesRule.html @@ -10,6 +10,7 @@ 0% { top: 0px; } 100% { top: 200px; } } + @keyframes empty {}