From 915c8725aee035d0a602ae946795c0b1f5c7315e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 4 Jun 2018 20:57:49 +0200 Subject: [PATCH] style: Remove PropertyId::name. It's only used for the error path in property parsing, so most of the time is not useful. Use the just-introduced NonCustomPropertyId::name to preserve the alias name, which we were doing by passing the name around. Bug: 1466645 Reviewed-by: xidorn MozReview-Commit-ID: 46xxZKCoeBB --- .../style/properties/declaration_block.rs | 6 +-- .../style/properties/properties.mako.rs | 44 +++++++++---------- .../style/stylesheets/keyframes_rule.rs | 4 +- components/style/stylesheets/supports_rule.rs | 29 ++++++------ components/style_traits/lib.rs | 6 ++- 5 files changed, 47 insertions(+), 42 deletions(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index a66058caf1e..3cdbf61de66 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -1116,9 +1116,7 @@ where let mut parser = Parser::new(&mut input); let start_position = parser.position(); parser.parse_entirely(|parser| { - let name = id.name().into(); - PropertyDeclaration::parse_into(declarations, id, name, &context, parser) - .map_err(|e| e.into()) + PropertyDeclaration::parse_into(declarations, id, &context, parser) }).map_err(|err| { let location = err.location; let error = ContextualParseError::UnsupportedPropertyDeclaration( @@ -1169,7 +1167,7 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> { } }; input.parse_until_before(Delimiter::Bang, |input| { - PropertyDeclaration::parse_into(self.declarations, id, name, self.context, input) + PropertyDeclaration::parse_into(self.declarations, id, self.context, input) })?; let importance = match input.try(parse_important) { Ok(()) => Importance::Important, diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e2078853844..5133cd6ae2d 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -23,7 +23,7 @@ use std::fmt::{self, Write}; use std::mem::{self, ManuallyDrop}; #[cfg(feature = "servo")] use cssparser::RGBA; -use cssparser::{CowRcStr, Parser, TokenSerializationType, serialize_identifier}; +use cssparser::{Parser, TokenSerializationType}; use cssparser::ParserInput; #[cfg(feature = "servo")] use euclid::SideOffsets2D; use context::QuirksMode; @@ -1765,21 +1765,6 @@ impl PropertyId { } } - /// Returns the name of the property without CSS escaping. - pub fn name(&self) -> Cow<'static, str> { - match *self { - PropertyId::ShorthandAlias(id, _) | - PropertyId::Shorthand(id) => id.name().into(), - PropertyId::LonghandAlias(id, _) | - PropertyId::Longhand(id) => id.name().into(), - PropertyId::Custom(ref name) => { - let mut s = String::new(); - write!(&mut s, "--{}", name).unwrap(); - s.into() - } - } - } - fn non_custom_id(&self) -> Option { Some(match *self { PropertyId::Custom(_) => return None, @@ -2042,13 +2027,13 @@ impl PropertyDeclaration { pub fn parse_into<'i, 't>( declarations: &mut SourcePropertyDeclaration, id: PropertyId, - name: CowRcStr<'i>, context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result<(), ParseError<'i>> { assert!(declarations.is_empty()); debug_assert!(id.allowed_in(context), "{:?}", id); + let non_custom_id = id.non_custom_id(); let start = input.state(); match id { PropertyId::Custom(property_name) => { @@ -2059,7 +2044,10 @@ impl PropertyDeclaration { Ok(keyword) => DeclaredValueOwned::CSSWideKeyword(keyword), Err(()) => match ::custom_properties::SpecifiedValue::parse(input) { Ok(value) => DeclaredValueOwned::Value(value), - Err(e) => return Err(StyleParseErrorKind::new_invalid(name, e)), + Err(e) => return Err(StyleParseErrorKind::new_invalid( + format!("--{}", property_name), + e, + )), } }; declarations.push(PropertyDeclaration::Custom(CustomDeclaration { @@ -2084,7 +2072,10 @@ impl PropertyDeclaration { input.reset(&start); let (first_token_type, css) = ::custom_properties::parse_non_custom_with_var(input).map_err(|e| { - StyleParseErrorKind::new_invalid(name, e) + StyleParseErrorKind::new_invalid( + non_custom_id.unwrap().name(), + e, + ) })?; Ok(PropertyDeclaration::WithVariables(VariableDeclaration { id, @@ -2096,7 +2087,10 @@ impl PropertyDeclaration { }), })) } else { - Err(StyleParseErrorKind::new_invalid(name, err)) + Err(StyleParseErrorKind::new_invalid( + non_custom_id.unwrap().name(), + err, + )) } }) }).map(|declaration| { @@ -2130,7 +2124,10 @@ impl PropertyDeclaration { input.reset(&start); let (first_token_type, css) = ::custom_properties::parse_non_custom_with_var(input).map_err(|e| { - StyleParseErrorKind::new_invalid(name, e) + StyleParseErrorKind::new_invalid( + non_custom_id.unwrap().name(), + e, + ) })?; let unparsed = Arc::new(UnparsedValue { css: css.into_owned(), @@ -2152,7 +2149,10 @@ impl PropertyDeclaration { } Ok(()) } else { - Err(StyleParseErrorKind::new_invalid(name, err)) + Err(StyleParseErrorKind::new_invalid( + non_custom_id.unwrap().name(), + err, + )) } }) } diff --git a/components/style/stylesheets/keyframes_rule.rs b/components/style/stylesheets/keyframes_rule.rs index afe797c12ff..5f39267ac7b 100644 --- a/components/style/stylesheets/keyframes_rule.rs +++ b/components/style/stylesheets/keyframes_rule.rs @@ -623,12 +623,12 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for KeyframeDeclarationParser<'a, 'b> { let id = match PropertyId::parse(&name, self.context) { Ok(id) => id, Err(()) => return Err(input.new_custom_error( - StyleParseErrorKind::UnknownProperty(name.clone()) + StyleParseErrorKind::UnknownProperty(name) )), }; // TODO(emilio): Shouldn't this use parse_entirely? - PropertyDeclaration::parse_into(self.declarations, id, name, self.context, input)?; + PropertyDeclaration::parse_into(self.declarations, id, self.context, input)?; // In case there is still unparsed text in the declaration, we should // roll back. diff --git a/components/style/stylesheets/supports_rule.rs b/components/style/stylesheets/supports_rule.rs index 5e2739d153d..f8c3235f295 100644 --- a/components/style/stylesheets/supports_rule.rs +++ b/components/style/stylesheets/supports_rule.rs @@ -316,20 +316,23 @@ impl Declaration { let mut input = ParserInput::new(&self.0); let mut input = Parser::new(&mut input); input.parse_entirely(|input| -> Result<(), CssParseError<()>> { - let prop = input.expect_ident_cloned().unwrap(); - input.expect_colon().unwrap(); + let prop = input.expect_ident_cloned().unwrap(); + input.expect_colon().unwrap(); - let id = PropertyId::parse(&prop, context) - .map_err(|_| input.new_custom_error(()))?; + let id = PropertyId::parse(&prop, context) + .map_err(|_| input.new_custom_error(()))?; - let mut declarations = SourcePropertyDeclaration::new(); - input.parse_until_before(Delimiter::Bang, |input| { - PropertyDeclaration::parse_into(&mut declarations, id, prop, &context, input) - .map_err(|_| input.new_custom_error(())) - })?; - let _ = input.try(parse_important); - Ok(()) - }) - .is_ok() + let mut declarations = SourcePropertyDeclaration::new(); + input.parse_until_before(Delimiter::Bang, |input| { + PropertyDeclaration::parse_into( + &mut declarations, + id, + &context, + input, + ).map_err(|_| input.new_custom_error(())) + })?; + let _ = input.try(parse_important); + Ok(()) + }).is_ok() } } diff --git a/components/style_traits/lib.rs b/components/style_traits/lib.rs index f232b157cf9..b1eaf4e086f 100644 --- a/components/style_traits/lib.rs +++ b/components/style_traits/lib.rs @@ -177,7 +177,11 @@ pub enum ValueParseErrorKind<'i> { impl<'i> StyleParseErrorKind<'i> { /// Create an InvalidValue parse error - pub fn new_invalid(name: CowRcStr<'i>, value_error: ParseError<'i>) -> ParseError<'i> { + pub fn new_invalid(name: S, value_error: ParseError<'i>) -> ParseError<'i> + where + S: Into>, + { + let name = name.into(); let variant = match value_error.kind { cssparser::ParseErrorKind::Custom(StyleParseErrorKind::ValueError(e)) => { match e {