From acdd8aa99a8bc57aab277e25faa25f21be795e53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 1 Jun 2017 02:24:58 +0200 Subject: [PATCH] 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 --- .../style/properties/declaration_block.rs | 75 +++++++++++++++---- 1 file changed, 60 insertions(+), 15 deletions(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 32b18a3d3cb..7a349db0ff3 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -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() {