style: Insert declarations last while parsing.

The CSSOM spec forces us to reuse the slot, so we need to conditionally keep the
previous behavior.

This fixes bug 1369198.

MozReview-Commit-ID: LM9mK6ngZ4e
This commit is contained in:
Emilio Cobos Álvarez 2017-06-01 02:24:58 +02:00
parent cf71a0cd96
commit acdd8aa99a
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C

View file

@ -257,22 +257,30 @@ impl PropertyDeclarationBlock {
}
/// Adds or overrides the declaration for a given property in this block,
/// **except** if an existing declaration for the same property is more important.
/// **except** if an existing declaration for the same property is more
/// important.
///
/// Always ensures that the property declaration is at the end.
pub fn extend(&mut self, drain: SourcePropertyDeclarationDrain, importance: Importance) {
self.extend_common(drain, importance, false);
}
/// Adds or overrides the declaration for a given property in this block,
/// **even** if an existing declaration for the same property is more important.
/// **even** if an existing declaration for the same property is more
/// important, and reuses the same position in the block.
///
/// Return whether anything changed.
/// Returns whether anything changed.
pub fn extend_reset(&mut self, drain: SourcePropertyDeclarationDrain,
importance: Importance) -> bool {
self.extend_common(drain, importance, true)
}
fn extend_common(&mut self, mut drain: SourcePropertyDeclarationDrain,
importance: Importance, overwrite_more_important: bool) -> bool {
fn extend_common(
&mut self,
mut drain: SourcePropertyDeclarationDrain,
importance: Importance,
overwrite_more_important_and_reuse_slot: bool,
) -> bool {
let all_shorthand_len = match drain.all_shorthand {
AllShorthand::NotSet => 0,
AllShorthand::CSSWideKeyword(_) |
@ -285,20 +293,32 @@ impl PropertyDeclarationBlock {
let mut changed = false;
for decl in &mut drain.declarations {
changed |= self.push_common(decl, importance, overwrite_more_important);
changed |= self.push_common(
decl,
importance,
overwrite_more_important_and_reuse_slot,
);
}
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);
changed |= self.push_common(
decl,
importance,
overwrite_more_important_and_reuse_slot,
);
}
}
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);
changed |= self.push_common(
decl,
importance,
overwrite_more_important_and_reuse_slot,
);
}
}
}
@ -306,28 +326,38 @@ impl PropertyDeclarationBlock {
}
/// Adds or overrides the declaration for a given property in this block,
/// **except** if an existing declaration for the same property is more important.
/// **except** if an existing declaration for the same property is more
/// important.
///
/// Ensures that, if inserted, it's inserted at the end of the declaration
/// block.
pub fn push(&mut self, declaration: PropertyDeclaration, importance: Importance) {
self.push_common(declaration, importance, false);
}
fn push_common(&mut self, declaration: PropertyDeclaration, importance: Importance,
overwrite_more_important: bool) -> bool {
fn push_common(
&mut self,
declaration: PropertyDeclaration,
importance: Importance,
overwrite_more_important_and_reuse_slot: bool
) -> bool {
let definitely_new = if let PropertyDeclarationId::Longhand(id) = declaration.id() {
!self.longhands.contains(id)
} else {
false // For custom properties, always scan
};
if !definitely_new {
for slot in &mut *self.declarations {
let mut index_to_remove = None;
for (i, slot) in self.declarations.iter_mut().enumerate() {
if slot.0.id() == declaration.id() {
match (slot.1, importance) {
(Importance::Normal, Importance::Important) => {
self.important_count += 1;
}
(Importance::Important, Importance::Normal) => {
if overwrite_more_important {
if overwrite_more_important_and_reuse_slot {
self.important_count -= 1;
} else {
return false
@ -337,10 +367,25 @@ impl PropertyDeclarationBlock {
return false;
}
}
*slot = (declaration, importance);
return true
if overwrite_more_important_and_reuse_slot {
*slot = (declaration, importance);
return true;
}
// NOTE(emilio): We could avoid this and just override for
// properties not affected by logical props, but it's not
// clear it's worth it given the `definitely_new` check.
index_to_remove = Some(i);
break;
}
}
if let Some(index) = index_to_remove {
self.declarations.remove(index);
self.declarations.push((declaration, importance));
return true;
}
}
if let PropertyDeclarationId::Longhand(id) = declaration.id() {