Auto merge of #20582 - upsuper:cssom-append, r=emilio

Have CSSOM append rather than replace slot in declaration block

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.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20582)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2018-04-07 09:18:28 -04:00 committed by GitHub
commit ccc9d1c4c2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -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 {