style: Move property allowance tests to PropertyId::parse_into.

It's not only more consistent (since we have a proper ParserContext there), but
also fixes a bunch of bugs where Gecko accidentally exposes and allows setting
internal state because of conversions from nsCSSPropertyID to PropertyId.

This adds the extra complexity of caring about aliases for longer, but that's
probably not a big deal in practice, since we also have PropertyDeclarationId.

MozReview-Commit-ID: C2Js8PfloxQ
This commit is contained in:
Emilio Cobos Álvarez 2017-11-21 13:33:35 +01:00
parent 5905f8d3ea
commit 8de554f334
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
9 changed files with 137 additions and 135 deletions

View file

@ -9,7 +9,7 @@ use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule, Source
use error_reporting::{NullReporter, ContextualParseError, ParseErrorReporter};
use parser::{ParserContext, ParserErrorContext};
use properties::{DeclarationSource, Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId};
use properties::{PropertyDeclarationId, PropertyParserContext, LonghandId, SourcePropertyDeclaration};
use properties::{PropertyDeclarationId, LonghandId, SourcePropertyDeclaration};
use properties::LonghandIdSet;
use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction;
use servo_arc::Arc;
@ -581,19 +581,22 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for KeyframeDeclarationParser<'a, 'b> {
type Declaration = ();
type Error = StyleParseErrorKind<'i>;
fn parse_value<'t>(&mut self, name: CowRcStr<'i>, input: &mut Parser<'i, 't>)
-> Result<(), ParseError<'i>> {
let property_context = PropertyParserContext::new(self.context);
let id = PropertyId::parse(&name, Some(&property_context)).map_err(|()| {
fn parse_value<'t>(
&mut self,
name: CowRcStr<'i>,
input: &mut Parser<'i, 't>,
) -> Result<(), ParseError<'i>> {
let id = PropertyId::parse(&name).map_err(|()| {
input.new_custom_error(StyleParseErrorKind::UnknownProperty(name.clone()))
})?;
match PropertyDeclaration::parse_into(self.declarations, id, name, self.context, input) {
Ok(()) => {
// In case there is still unparsed text in the declaration, we should roll back.
input.expect_exhausted().map_err(|e| e.into())
}
Err(e) => Err(e.into())
}
// TODO(emilio): Shouldn't this use parse_entirely?
PropertyDeclaration::parse_into(self.declarations, id, name, self.context, input)?;
// In case there is still unparsed text in the declaration, we should
// roll back.
input.expect_exhausted()?;
Ok(())
}
}

View file

@ -9,7 +9,7 @@ use cssparser::{ParseError as CssParseError, ParserInput};
#[cfg(feature = "gecko")]
use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
use parser::ParserContext;
use properties::{PropertyId, PropertyDeclaration, PropertyParserContext, SourcePropertyDeclaration};
use properties::{PropertyId, PropertyDeclaration, SourcePropertyDeclaration};
use selectors::parser::SelectorParseErrorKind;
use servo_arc::Arc;
use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
@ -259,9 +259,8 @@ impl Declaration {
let prop = input.expect_ident().unwrap().as_ref().to_owned();
input.expect_colon().unwrap();
let property_context = PropertyParserContext::new(&context);
let id = PropertyId::parse(&prop, Some(&property_context))
.map_err(|()| input.new_custom_error(()))?;
let id = PropertyId::parse(&prop)
.map_err(|_| input.new_custom_error(()))?;
let mut declarations = SourcePropertyDeclaration::new();
input.parse_until_before(Delimiter::Bang, |input| {