From bcf3754a3f00fa0955aefc2d2ee6153dc9f19f3a Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Sat, 7 Apr 2018 09:18:28 -0400 Subject: [PATCH] servo: Merge #20582 - Have CSSOM append rather than replace slot in declaration block (from upsuper:cssom-append); r=emilio The early return for identical setting in importance matching as well as the comment before `index_to_remove` are removed because the order is web-exposing regardless of whether it's from CSSOM or parsing. e.g. `top: 1px; left: 2px; top: 1px;` is effectively `left: 2px; top: 1px;`, not `top: 1px; left: 2px;`. This is patch for [bug 1415330](https://bugzilla.mozilla.org/show_bug.cgi?id=1415330), for spec change in w3c/csswg-drafts#2516. And corresponding test will be added in w3c/web-platform-tests#10354. Source-Repo: https://github.com/servo/servo Source-Revision: ccc9d1c4c2877ebc82158b91dc27e1be74fae237 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 9c326db0bca76383e0c47c53baa0fd7f20481130 --- .../style/properties/declaration_block.rs | 60 +++++++------------ 1 file changed, 20 insertions(+), 40 deletions(-) diff --git a/servo/components/style/properties/declaration_block.rs b/servo/components/style/properties/declaration_block.rs index c2d7b42ca6ec..58c4bd97ee4a 100644 --- a/servo/components/style/properties/declaration_block.rs +++ b/servo/components/style/properties/declaration_block.rs @@ -518,53 +518,33 @@ impl PropertyDeclarationBlock { } let important = self.declarations_importance.get(i as u32); - match (important, importance.important()) { - (false, true) => {} - - (true, false) => { - // For declarations set from the OM, more-important - // declarations are overridden. - if !matches!(source, DeclarationSource::CssOm) { - return false - } - } - _ => if *slot == declaration { - return false; - } + // For declarations from parsing, non-important declarations + // shouldn't override existing important one. + if important && !importance.important() && + matches!(source, DeclarationSource::Parsing) { + return true; } - match source { - // CSSOM preserves the declaration position, and - // overrides importance. - DeclarationSource::CssOm => { - *slot = declaration; - self.declarations_importance.set(i as u32, importance.important()); - return true; - } - DeclarationSource::Parsing => { - // As a compatibility hack, specially on Android, - // don't allow to override a prefixed webkit display - // value with an unprefixed version from parsing - // code. - // - // TODO(emilio): Unship. - if let PropertyDeclaration::Display(old_display) = *slot { - use properties::longhands::display::computed_value::T as display; + if matches!(source, DeclarationSource::Parsing) { + // As a compatibility hack, specially on Android, + // don't allow to override a prefixed webkit display + // value with an unprefixed version from parsing + // code. + // + // TODO(emilio): Unship. + if let PropertyDeclaration::Display(old_display) = *slot { + use properties::longhands::display::computed_value::T as display; - if let PropertyDeclaration::Display(new_display) = declaration { - if display::should_ignore_parsed_value(old_display, new_display) { - return false; - } + if let PropertyDeclaration::Display(new_display) = declaration { + if display::should_ignore_parsed_value(old_display, new_display) { + return false; } } - - // 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; } } + + index_to_remove = Some(i); + break; } if let Some(index) = index_to_remove {