Bug 1369228: Respect `all` shorthand position when parsing property declaration blocks. r?heycam
MozReview-Commit-ID: 457spIBP4df
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -279,17 +279,17 @@ impl PropertyDeclarationBlock {
AllShorthand::WithVariables(_) => ShorthandId::All.longhands().len()
};
let push_calls_count = drain.declarations.len() + all_shorthand_len;
// With deduplication the actual length increase may be less than this.
self.declarations.reserve(push_calls_count);
let mut changed = false;
- for decl in &mut drain.declarations {
+ for decl in (&mut drain.declarations).take(drain.all_shorthand_position) {
changed |= self.push_common(decl, importance, overwrite_more_important);
}
match drain.all_shorthand {
AllShorthand::NotSet => {}
AllShorthand::CSSWideKeyword(keyword) => {
for &id in ShorthandId::All.longhands() {
let decl = PropertyDeclaration::CSSWideKeyword(id, keyword);
changed |= self.push_common(decl, importance, overwrite_more_important);
@@ -297,16 +297,19 @@ impl PropertyDeclarationBlock {
}
AllShorthand::WithVariables(unparsed) => {
for &id in ShorthandId::All.longhands() {
let decl = PropertyDeclaration::WithVariables(id, unparsed.clone());
changed |= self.push_common(decl, importance, overwrite_more_important);
}
}
}
+ for decl in drain.declarations {
+ changed |= self.push_common(decl, importance, overwrite_more_important);
+ }
changed
}
/// Adds or overrides the declaration for a given property in this block,
/// **except** if an existing declaration for the same property is more important.
pub fn push(&mut self, declaration: PropertyDeclaration, importance: Importance) {
self.push_common(declaration, importance, false);
}
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -208,22 +208,22 @@ pub mod shorthands {
// definition.
input.look_for_var_functions();
let start = input.position();
while let Ok(_) = input.next() {} // Look for var()
if input.seen_var_functions() {
input.reset(start);
let (first_token_type, css) = try!(
::custom_properties::parse_non_custom_with_var(input));
- declarations.all_shorthand = AllShorthand::WithVariables(Arc::new(UnparsedValue {
+ declarations.set_all_shorthand(AllShorthand::WithVariables(Arc::new(UnparsedValue {
css: css.into_owned(),
first_token_type: first_token_type,
url_data: context.url_data.clone(),
from_shorthand: Some(ShorthandId::All),
- }));
+ })));
Ok(())
} else {
Err(())
}
}
}
}
@@ -1438,17 +1438,17 @@ impl PropertyDeclaration {
}
% endif
${property_pref_check(shorthand)}
match input.try(|i| CSSWideKeyword::parse(context, i)) {
Ok(keyword) => {
% if shorthand.name == "all":
- declarations.all_shorthand = AllShorthand::CSSWideKeyword(keyword);
+ declarations.set_all_shorthand(AllShorthand::CSSWideKeyword(keyword));
% else:
% for sub_property in shorthand.sub_properties:
declarations.push(PropertyDeclaration::CSSWideKeyword(
LonghandId::${sub_property.camel_case},
keyword,
));
% endfor
% endif
@@ -1475,56 +1475,71 @@ type SourcePropertyDeclarationArray =
/// A stack-allocated vector of `PropertyDeclaration`
/// large enough to parse one CSS `key: value` declaration.
/// (Shorthands expand to multiple `PropertyDeclaration`s.)
pub struct SourcePropertyDeclaration {
declarations: ::arrayvec::ArrayVec<SourcePropertyDeclarationArray>,
/// Stored separately to keep MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL smaller.
all_shorthand: AllShorthand,
+
+ /// The position all_shorthand would be in `declarations`.
+ all_shorthand_position: usize,
}
impl SourcePropertyDeclaration {
/// Create one. It’s big, try not to move it around.
#[inline]
pub fn new() -> Self {
SourcePropertyDeclaration {
declarations: ::arrayvec::ArrayVec::new(),
all_shorthand: AllShorthand::NotSet,
+ all_shorthand_position: 0,
}
}
/// Similar to Vec::drain: leaves this empty when the return value is dropped.
pub fn drain(&mut self) -> SourcePropertyDeclarationDrain {
SourcePropertyDeclarationDrain {
declarations: self.declarations.drain(..),
all_shorthand: mem::replace(&mut self.all_shorthand, AllShorthand::NotSet),
+ all_shorthand_position: self.all_shorthand_position,
}
}
+ fn set_all_shorthand(&mut self, shorthand: AllShorthand) {
+ debug_assert!(!matches!(shorthand, AllShorthand::NotSet));
+
+ let pos = self.declarations.len();
+ self.all_shorthand = shorthand;
+ self.all_shorthand_position = pos;
+ }
+
/// Reset to initial state
pub fn clear(&mut self) {
self.declarations.clear();
self.all_shorthand = AllShorthand::NotSet;
+ self.all_shorthand_position = 0;
}
fn is_empty(&self) -> bool {
self.declarations.is_empty() && matches!(self.all_shorthand, AllShorthand::NotSet)
}
fn push(&mut self, declaration: PropertyDeclaration) {
let over_capacity = self.declarations.push(declaration).is_some();
debug_assert!(!over_capacity);
}
}
/// Return type of SourcePropertyDeclaration::drain
pub struct SourcePropertyDeclarationDrain<'a> {
declarations: ::arrayvec::Drain<'a, SourcePropertyDeclarationArray>,
all_shorthand: AllShorthand,
+ all_shorthand_position: usize,
}
enum AllShorthand {
NotSet,
CSSWideKeyword(CSSWideKeyword),
WithVariables(Arc<UnparsedValue>)
}