Auto merge of #18352 - jdm:serialize-fun, r=emilio

Make serialization match Gecko in a few corner cases

This addresses the testcases from https://bugzilla.mozilla.org/show_bug.cgi?id=1345218. Gecko differs from the specification by doing the following:
* reusing longhands that have previously been serialized in order to serialize shorthands
* immediately breaking out of the shorthand loop for the current property as soon as a shorthand is successfully serialized

https://github.com/w3c/csswg-drafts/issues/1774 is filed to track ways that the standard could be made more clear on these points.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [bug 1345218](https://bugzilla.mozilla.org/show_bug.cgi?id=1345218).
- [X] There are tests for these changes

<!-- 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/18352)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-09-13 10:51:00 -05:00 committed by GitHub
commit e50341d4a9
3 changed files with 68 additions and 9 deletions

View file

@ -657,10 +657,6 @@ impl ToCss for PropertyDeclarationBlock {
if is_system_font {
for (longhand, importance) in self.declaration_importance_iter() {
if already_serialized.contains(longhand.id()) {
continue;
}
if longhand.get_system().is_some() || longhand.is_default_line_height() {
current_longhands.push(longhand);
if found_system.is_none() {
@ -673,10 +669,6 @@ impl ToCss for PropertyDeclarationBlock {
}
} else {
for (longhand, importance) in self.declaration_importance_iter() {
if already_serialized.contains(longhand.id()) {
continue;
}
if longhand.id().is_longhand_of(shorthand) {
current_longhands.push(longhand);
if importance.important() {
@ -771,6 +763,13 @@ impl ToCss for PropertyDeclarationBlock {
// Substep 9
already_serialized.insert(current_longhand.id());
}
// FIXME(https://github.com/w3c/csswg-drafts/issues/1774)
// The specification does not include an instruction to abort
// the shorthand loop at this point, but doing so both matches
// Gecko and makes sense since shorthands are checked in
// preferred order.
break;
}
}

View file

@ -476,15 +476,28 @@ impl LonghandId {
// algorithms and what not, I guess.
<%
longhand_to_shorthand_map = {}
num_sub_properties = {}
for shorthand in data.shorthands:
num_sub_properties[shorthand.camel_case] = len(shorthand.sub_properties)
for sub_property in shorthand.sub_properties:
if sub_property.ident not in longhand_to_shorthand_map:
longhand_to_shorthand_map[sub_property.ident] = []
longhand_to_shorthand_map[sub_property.ident].append(shorthand.camel_case)
def preferred_order(x, y):
# Since we want properties in order from most subproperties to least,
# reverse the arguments to cmp from the expected order.
result = cmp(num_sub_properties.get(y, 0), num_sub_properties.get(x, 0))
if result:
return result
# Fall back to lexicographic comparison.
return cmp(x, y)
# Sort the lists of shorthand properties according to preferred order:
# https://drafts.csswg.org/cssom/#concept-shorthands-preferred-order
for shorthand_list in longhand_to_shorthand_map.itervalues():
shorthand_list.sort()
shorthand_list.sort(cmp=preferred_order)
%>
// based on lookup results for each longhand, create result arrays