Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-23 12:51:06 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1911353 - Unify how we reject !important in keyframe and @position-try. r=dshin

Browse Source 
This was more the kind of thing I meant, and allows us to get rid of the
keyframe-rule-specific parser.

Differential Revision: https://phabricator.services.mozilla.com/D218488
This commit is contained in:
Emilio Cobos Álvarez 2024-08-16 13:14:05 +00:00
parent 543946adb5
commit 9c457b174d
10 changed files with 31 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
dom/locales/en-US/chrome/layout/css.properties
View File

@ -47,7 +47,7 @@ PEExpectedNoneOrURL=Expected ‘none’ or URL but found ‘%1$S’.
PEExpectedNoneOrURLOrFilterFunction=Expected ‘none’, URL, or filter function but found ‘%1$S’.
PEDisallowedImportRule=@import rules are not yet valid in constructed stylesheets.
PENeverMatchingHostSelector=:host selector in ‘%S’ is not featureless and will never match. Maybe you intended to use :host()?
PEPositionTryImportantDeclError=Property cannot be declared as !important in a @position-try rule.
PEImportantDeclError=Property cannot be declared as !important in this context.
TooLargeDashedRadius=Border radius is too large for ‘dashed’ style (the limit is 100000px). Rendering as solid.
TooLargeDottedRadius=Border radius is too large for ‘dotted’ style (the limit is 100000px). Rendering as solid.

2
layout/style/test/test_css_parse_error_smoketest.html
View File

@ -127,7 +127,7 @@
},
{
css: "@position-try --foo { width: 10px !important; }",
error: "Property cannot be declared as !important in a @position-try rule. Declaration dropped."
error: "Property cannot be declared as !important in this context. Declaration dropped."
},
];

6
servo/components/style/error_reporting.rs
View File

@ -32,8 +32,6 @@ pub enum ContextualParseError<'a> {
InvalidKeyframeRule(&'a str, ParseError<'a>),
/// A font feature values rule was not valid.
InvalidFontFeatureValuesRule(&'a str, ParseError<'a>),
/// A keyframe property declaration was not recognized.
UnsupportedKeyframePropertyDeclaration(&'a str, ParseError<'a>),
/// A rule was invalid for some reason.
InvalidRule(&'a str, ParseError<'a>),
/// A rule was not recognized.
@ -175,10 +173,6 @@ impl<'a> fmt::Display for ContextualParseError<'a> {
write!(f, "Invalid font feature value rule: '{}', ", rule)?;
parse_error_to_str(err, f)
},
ContextualParseError::UnsupportedKeyframePropertyDeclaration(decl, ref err) => {
write!(f, "Unsupported keyframe property declaration: '{}', ", decl)?;
parse_error_to_str(err, f)
},
ContextualParseError::InvalidRule(rule, ref err) => {
write!(f, "Invalid rule: '{}', ", rule)?;
parse_error_to_str(err, f)

6
servo/components/style/parser.rs
View File

@ -131,6 +131,12 @@ impl<'a> ParserContext<'a> {
self.nesting_context.rule_types.contains(CssRuleType::Page)
}
/// Returns whether !important declarations are forbidden.
#[inline]
pub fn allows_important_declarations(&self) -> bool {
!self.nesting_context.rule_types.intersects(CssRuleTypes::IMPORTANT_FORBIDDEN)
}
/// Get the rule type, which assumes that one is available.
pub fn rule_types(&self) -> CssRuleTypes {
self.nesting_context.rule_types

18
servo/components/style/properties/declaration_block.rs
View File

@ -1455,18 +1455,14 @@ impl<'i> DeclarationParserState<'i> {
PropertyDeclaration::parse_into(&mut self.declarations, id, context, input)
})?;
self.importance = match input.try_parse(parse_important) {
Ok(()) => Importance::Important,
Ok(()) => {
if !context.allows_important_declarations() {
return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedImportantDeclaration));
}
Importance::Important
},
Err(_) => Importance::Normal,
};
if context
.nesting_context
.rule_types
.contains(CssRuleType::PositionTry) &&
matches!(self.importance, Importance::Important)
{
return Err(input
.new_custom_error(StyleParseErrorKind::PositionTryUnexpectedImportantDeclaration));
}
// In case there is still unparsed text in the declaration, we should roll back.
input.expect_exhausted()?;
self.output_block
@ -1623,7 +1619,7 @@ fn report_one_css_error<'i>(
// the error to be more specific.
if !matches!(
error.kind,
ParseErrorKind::Custom(StyleParseErrorKind::PositionTryUnexpectedImportantDeclaration)
ParseErrorKind::Custom(StyleParseErrorKind::UnexpectedImportantDeclaration)
) {
error = match *property {
PropertyId::Custom(ref c) => {

6
servo/components/style/stylesheets/font_feature_values_rule.rs
View File

@ -422,9 +422,9 @@ macro_rules! font_feature_values_blocks {
while let Some(declaration) = iter.next() {
if let Err((error, slice)) = declaration {
let location = error.location;
let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration(
slice, error
);
// TODO(emilio): Maybe add a more specific error kind for
// font-feature-values descriptors.
let error = ContextualParseError::UnsupportedPropertyDeclaration(slice, error, &[]);
self.context.log_css_error(location, error);
}
}

97
servo/components/style/stylesheets/keyframes_rule.rs
View File

@ -11,8 +11,8 @@ use crate::properties::{
animation_composition::single_value::SpecifiedValue as SpecifiedComposition,
transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction,
},
Importance, LonghandId, PropertyDeclaration, PropertyDeclarationBlock, PropertyDeclarationId,
PropertyDeclarationIdSet, PropertyId, SourcePropertyDeclaration,
parse_property_declaration_list, LonghandId, PropertyDeclaration, PropertyDeclarationBlock,
PropertyDeclarationId, PropertyDeclarationIdSet,
};
use crate::shared_lock::{DeepCloneParams, DeepCloneWithLock, SharedRwLock, SharedRwLockReadGuard};
use crate::shared_lock::{Locked, ToCssWithGuard};
@ -21,7 +21,7 @@ use crate::stylesheets::rule_parser::VendorPrefix;
use crate::stylesheets::{CssRuleType, StylesheetContents};
use crate::values::{serialize_percentage, KeyframesName};
use cssparser::{
parse_one_rule, AtRuleParser, CowRcStr, DeclarationParser, Parser, ParserInput, ParserState,
parse_one_rule, AtRuleParser, DeclarationParser, Parser, ParserInput, ParserState,
QualifiedRuleParser, RuleBodyItemParser, RuleBodyParser, SourceLocation, Token,
};
use servo_arc::Arc;
@ -227,11 +227,9 @@ impl Keyframe {
let mut input = ParserInput::new(css);
let mut input = Parser::new(&mut input);
let mut declarations = SourcePropertyDeclaration::default();
let mut rule_parser = KeyframeListParser {
context: &mut context,
shared_lock: &lock,
declarations: &mut declarations,
};
parse_one_rule(&mut input, &mut rule_parser)
}
@ -529,7 +527,6 @@ impl KeyframesAnimation {
struct KeyframeListParser<'a, 'b> {
context: &'a mut ParserContext<'b>,
shared_lock: &'a SharedRwLock,
declarations: &'a mut SourcePropertyDeclaration,
}
/// Parses a keyframe list from CSS input.
@ -538,11 +535,9 @@ pub fn parse_keyframe_list<'a>(
input: &mut Parser,
shared_lock: &SharedRwLock,
) -> Vec<Arc<Locked<Keyframe>>> {
let mut declarations = SourcePropertyDeclaration::default();
let mut parser = KeyframeListParser {
context,
shared_lock,
declarations: &mut declarations,
};
RuleBodyParser::new(input, &mut parser)
.filter_map(Result::ok)
@ -587,33 +582,9 @@ impl<'a, 'b, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a, 'b> {
start: &ParserState,
input: &mut Parser<'i, 't>,
) -> Result<Self::QualifiedRule, ParseError<'i>> {
let mut block = PropertyDeclarationBlock::new();
let declarations = &mut self.declarations;
self.context
.nest_for_rule(CssRuleType::Keyframe, |context| {
let mut parser = KeyframeDeclarationParser {
context: &context,
declarations,
};
let mut iter = RuleBodyParser::new(input, &mut parser);
while let Some(declaration) = iter.next() {
match declaration {
Ok(()) => {
block.extend(iter.parser.declarations.drain(), Importance::Normal);
},
Err((error, slice)) => {
iter.parser.declarations.clear();
let location = error.location;
let error =
ContextualParseError::UnsupportedKeyframePropertyDeclaration(
slice, error,
);
context.log_css_error(location, error);
},
}
// `parse_important` is not called here, `!important` is not allowed in keyframe blocks.
}
});
let block = self.context.nest_for_rule(CssRuleType::Keyframe, |p| {
parse_property_declaration_list(&p, input, &[])
});
Ok(Arc::new(self.shared_lock.wrap(Keyframe {
selector,
block: Arc::new(self.shared_lock.wrap(block)),
@ -632,59 +603,3 @@ impl<'a, 'b, 'i> RuleBodyItemParser<'i, Arc<Locked<Keyframe>>, StyleParseErrorKi
false
}
}
struct KeyframeDeclarationParser<'a, 'b: 'a> {
context: &'a ParserContext<'b>,
declarations: &'a mut SourcePropertyDeclaration,
}
/// Default methods reject all at rules.
impl<'a, 'b, 'i> AtRuleParser<'i> for KeyframeDeclarationParser<'a, 'b> {
type Prelude = ();
type AtRule = ();
type Error = StyleParseErrorKind<'i>;
}
impl<'a, 'b, 'i> QualifiedRuleParser<'i> for KeyframeDeclarationParser<'a, 'b> {
type Prelude = ();
type QualifiedRule = ();
type Error = StyleParseErrorKind<'i>;
}
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 id = match PropertyId::parse(&name, self.context) {
Ok(id) => id,
Err(()) => {
return Err(input.new_custom_error(StyleParseErrorKind::UnknownProperty(name)));
},
};
// TODO(emilio): Shouldn't this use parse_entirely?
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(())
}
}
impl<'a, 'b, 'i> RuleBodyItemParser<'i, (), StyleParseErrorKind<'i>>
for KeyframeDeclarationParser<'a, 'b>
{
fn parse_qualified(&self) -> bool {
false
}
fn parse_declarations(&self) -> bool {
true
}
}

3
servo/components/style/stylesheets/mod.rs
View File

@ -424,6 +424,9 @@ impl From<CssRuleType> for CssRuleTypes {
}
impl CssRuleTypes {
/// Rules where !important declarations are forbidden.
pub const IMPORTANT_FORBIDDEN: Self = Self(CssRuleType::PositionTry.bit() | CssRuleType::Keyframe.bit());
/// Returns whether the rule is in the current set.
#[inline]
pub fn contains(self, ty: CssRuleType) -> bool {

11
servo/components/style_traits/lib.rs
View File

@ -163,15 +163,8 @@ pub enum StyleParseErrorKind<'i> {
InvalidFilter(CowRcStr<'i>, Token<'i>),
/// The property declaration contained an invalid value.
OtherInvalidValue(CowRcStr<'i>),
/// The declaration contained an animation property, and we were parsing
/// this as a keyframe block (so that property should be ignored).
///
/// See: https://drafts.csswg.org/css-animations/#keyframes
AnimationPropertyInKeyframeBlock,
/// The property is not allowed within a page rule.
NotAllowedInPageRule,
/// `!important` declarations are disallowed in `@position-try`.
PositionTryUnexpectedImportantDeclaration,
/// `!important` declarations are disallowed in `@position-try` or keyframes.
UnexpectedImportantDeclaration,
}
impl<'i> From<ValueParseErrorKind<'i>> for StyleParseErrorKind<'i> {

8
servo/ports/geckolib/error_reporter.rs
View File

@ -196,7 +196,6 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
ContextualParseError::UnsupportedFontPaletteValuesDescriptor(s, err) |
ContextualParseError::InvalidKeyframeRule(s, err) |
ContextualParseError::InvalidFontFeatureValuesRule(s, err) |
ContextualParseError::UnsupportedKeyframePropertyDeclaration(s, err) |
ContextualParseError::InvalidRule(s, err) |
ContextualParseError::UnsupportedRule(s, err) |
ContextualParseError::UnsupportedViewportDescriptorDeclaration(s, err) |
@ -276,8 +275,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
StyleParseErrorKind::OtherInvalidValue(_) => {
(cstr!("PEValueParsingError"), Action::Drop)
},
StyleParseErrorKind::PositionTryUnexpectedImportantDeclaration => {
(cstr!("PEPositionTryImportantDeclError"), Action::Drop)
StyleParseErrorKind::UnexpectedImportantDeclaration => {
(cstr!("PEImportantDeclError"), Action::Drop)
},
_ => (cstr!("PEUnknownProperty"), Action::Drop),
},
@ -290,9 +289,6 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
ContextualParseError::InvalidKeyframeRule(..) => {
(cstr!("PEKeyframeBadName"), Action::Nothing)
},
ContextualParseError::UnsupportedKeyframePropertyDeclaration(..) => {
(cstr!("PEBadSelectorKeyframeRuleIgnored"), Action::Nothing)
},
ContextualParseError::InvalidRule(
_,
ParseError {