Bug 1466645: Remove PropertyId::name. r?xidorn
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.
MozReview-Commit-ID: 46xxZKCoeBB
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -1111,19 +1111,17 @@ where
parsing_mode,
quirks_mode,
);
let mut input = ParserInput::new(input);
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(
parser.slice_from(start_position), err);
let error_context = ParserErrorContext { error_reporter: error_reporter };
context.log_css_error(&error_context, location, error);
})
}
@@ -1164,17 +1162,17 @@ impl<'a, 'b, 'i> DeclarationParser<'i> f
return Err(input.new_custom_error(if is_non_mozilla_vendor_identifier(&name) {
StyleParseErrorKind::UnknownVendorProperty
} else {
StyleParseErrorKind::UnknownProperty(name)
}));
}
};
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,
Err(_) => Importance::Normal,
};
// In case there is still unparsed text in the declaration, we should roll back.
input.expect_exhausted()?;
Ok(importance)
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -18,17 +18,17 @@ use servo_arc::{Arc, UniqueArc};
use smallbitvec::SmallBitVec;
use std::borrow::Cow;
use std::{ops, ptr};
use std::cell::RefCell;
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;
use font_metrics::FontMetricsProvider;
#[cfg(feature = "gecko")] use gecko_bindings::bindings;
#[cfg(feature = "gecko")] use gecko_bindings::structs::{self, nsCSSPropertyID};
#[cfg(feature = "servo")] use logical_geometry::LogicalMargin;
#[cfg(feature = "servo")] use computed_values;
@@ -1760,31 +1760,16 @@ impl PropertyId {
PropertyId::ShorthandAlias(id, _) |
PropertyId::Shorthand(id) => Ok(id),
PropertyId::LonghandAlias(id, _) |
PropertyId::Longhand(id) => Err(PropertyDeclarationId::Longhand(id)),
PropertyId::Custom(ref name) => Err(PropertyDeclarationId::Custom(name)),
}
}
- /// 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<NonCustomPropertyId> {
Some(match *self {
PropertyId::Custom(_) => return None,
PropertyId::Shorthand(shorthand_id) => shorthand_id.into(),
PropertyId::Longhand(longhand_id) => longhand_id.into(),
PropertyId::ShorthandAlias(_, alias_id) => alias_id.into(),
PropertyId::LonghandAlias(_, alias_id) => alias_id.into(),
})
@@ -2037,34 +2022,37 @@ impl PropertyDeclaration {
/// > but does accept the `animation-play-state` property and interprets it specially.
///
/// This will not actually parse Importance values, and will always set things
/// to Importance::Normal. Parsing Importance values is the job of PropertyDeclarationParser,
/// we only set them here so that we don't have to reallocate
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) => {
// FIXME: fully implement https://github.com/w3c/csswg-drafts/issues/774
// before adding skip_whitespace here.
// This probably affects some test results.
let value = match input.try(|i| CSSWideKeyword::parse(i)) {
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 {
name: property_name,
value,
}));
Ok(())
}
@@ -2079,29 +2067,35 @@ impl PropertyDeclaration {
input.look_for_var_functions();
input.parse_entirely(|input| id.parse_value(context, input))
.or_else(|err| {
while let Ok(_) = input.next() {} // Look for var() after the error.
if input.seen_var_functions() {
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,
value: Arc::new(UnparsedValue {
css: css.into_owned(),
first_token_type: first_token_type,
url_data: context.url_data.clone(),
from_shorthand: None,
}),
}))
} else {
- Err(StyleParseErrorKind::new_invalid(name, err))
+ Err(StyleParseErrorKind::new_invalid(
+ non_custom_id.unwrap().name(),
+ err,
+ ))
}
})
}).map(|declaration| {
declarations.push(declaration)
})
}
PropertyId::ShorthandAlias(id, _) |
PropertyId::Shorthand(id) => {
@@ -2125,17 +2119,20 @@ impl PropertyDeclaration {
// Not using parse_entirely here: each ${shorthand.ident}::parse_into function
// needs to do so *before* pushing to `declarations`.
id.parse_into(declarations, context, input).or_else(|err| {
while let Ok(_) = input.next() {} // Look for var() after the error.
if input.seen_var_functions() {
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(),
first_token_type: first_token_type,
url_data: context.url_data.clone(),
from_shorthand: Some(id),
});
if id == ShorthandId::All {
@@ -2147,17 +2144,20 @@ impl PropertyDeclaration {
id,
value: unparsed.clone(),
})
)
}
}
Ok(())
} else {
- Err(StyleParseErrorKind::new_invalid(name, err))
+ Err(StyleParseErrorKind::new_invalid(
+ non_custom_id.unwrap().name(),
+ err,
+ ))
}
})
}
}
}
}
}
--- a/servo/components/style/stylesheets/keyframes_rule.rs
+++ b/servo/components/style/stylesheets/keyframes_rule.rs
@@ -618,22 +618,22 @@ impl<'a, 'b, 'i> DeclarationParser<'i> f
fn parse_value<'t>(
&mut self,
name: CowRcStr<'i>,
input: &mut Parser<'i, 't>,
) -> Result<(), ParseError<'i>> {
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.
input.expect_exhausted()?;
Ok(())
}
}
--- a/servo/components/style/stylesheets/supports_rule.rs
+++ b/servo/components/style/stylesheets/supports_rule.rs
@@ -311,25 +311,28 @@ impl Declaration {
///
/// <https://drafts.csswg.org/css-conditional-3/#support-definition>
pub fn eval(&self, context: &ParserContext) -> bool {
debug_assert_eq!(context.rule_type(), CssRuleType::Style);
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()
}
}
--- a/servo/components/style_traits/lib.rs
+++ b/servo/components/style_traits/lib.rs
@@ -172,17 +172,21 @@ pub enum ValueParseErrorKind<'i> {
/// An invalid token was encountered while parsing a color value.
InvalidColor(Token<'i>),
/// An invalid filter value was encountered.
InvalidFilter(Token<'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<S>(name: S, value_error: ParseError<'i>) -> ParseError<'i>
+ where
+ S: Into<CowRcStr<'i>>,
+ {
+ let name = name.into();
let variant = match value_error.kind {
cssparser::ParseErrorKind::Custom(StyleParseErrorKind::ValueError(e)) => {
match e {
ValueParseErrorKind::InvalidColor(token) => {
StyleParseErrorKind::InvalidColor(name, token)
}
ValueParseErrorKind::InvalidFilter(token) => {
StyleParseErrorKind::InvalidFilter(name, token)