From 67e80bc4f9b9f31a7e0e63f5d93573713a9fa1dd Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Wed, 2 Oct 2019 00:11:58 +0000 Subject: [PATCH] servo: Merge #18352 - Make serialization match Gecko in a few corner cases (from jdm:serialize-fun); r=emilio 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 Source-Repo: https://github.com/servo/servo Source-Revision: e50341d4a91beded42bdcdf37bfb8a7e53070234 UltraBlame original commit: db9ef4f3705b3a4541fef5093398ba43c110fc15 --- .../style/properties/declaration_block.rs | 15 +++--- .../style/properties/properties.mako.rs | 15 +++++- .../unit/style/properties/serialization.rs | 47 +++++++++++++++++++ 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/servo/components/style/properties/declaration_block.rs b/servo/components/style/properties/declaration_block.rs index 5ed8bb5bb3030..63bf6f6aea460 100644 --- a/servo/components/style/properties/declaration_block.rs +++ b/servo/components/style/properties/declaration_block.rs @@ -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 { already_serialized.insert(current_longhand.id()); } + + + + + + + break; } } diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index 0d9dc0e51cd7b..0599ab51b3b36 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -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 diff --git a/servo/tests/unit/style/properties/serialization.rs b/servo/tests/unit/style/properties/serialization.rs index 35e4aa7df071d..4e38a97446195 100644 --- a/servo/tests/unit/style/properties/serialization.rs +++ b/servo/tests/unit/style/properties/serialization.rs @@ -341,6 +341,53 @@ mod shorthand_serialization { mod border_shorthands { use super::*; + #[test] + fn border_top_and_color() { + let mut properties = Vec::new(); + properties.push(PropertyDeclaration::BorderTopWidth(BorderSideWidth::Length(Length::from_px(1.)))); + properties.push(PropertyDeclaration::BorderTopStyle(BorderStyle::solid)); + let c = Color::Numeric { + parsed: RGBA::new(255, 0, 0, 255), + authored: Some("green".to_string().into_boxed_str()) + }; + properties.push(PropertyDeclaration::BorderTopColor(c)); + let c = Color::Numeric { + parsed: RGBA::new(0, 255, 0, 255), + authored: Some("red".to_string().into_boxed_str()) + }; + properties.push(PropertyDeclaration::BorderTopColor(c.clone())); + properties.push(PropertyDeclaration::BorderBottomColor(c.clone())); + properties.push(PropertyDeclaration::BorderLeftColor(c.clone())); + properties.push(PropertyDeclaration::BorderRightColor(c.clone())); + + let serialization = shorthand_properties_to_string(properties); + assert_eq!(serialization, "border-top: 1px solid red; border-color: red;"); + } + + #[test] + fn border_color_and_top() { + let mut properties = Vec::new(); + let c = Color::Numeric { + parsed: RGBA::new(0, 255, 0, 255), + authored: Some("red".to_string().into_boxed_str()) + }; + properties.push(PropertyDeclaration::BorderTopColor(c.clone())); + properties.push(PropertyDeclaration::BorderBottomColor(c.clone())); + properties.push(PropertyDeclaration::BorderLeftColor(c.clone())); + properties.push(PropertyDeclaration::BorderRightColor(c.clone())); + + properties.push(PropertyDeclaration::BorderTopWidth(BorderSideWidth::Length(Length::from_px(1.)))); + properties.push(PropertyDeclaration::BorderTopStyle(BorderStyle::solid)); + let c = Color::Numeric { + parsed: RGBA::new(255, 0, 0, 255), + authored: Some("green".to_string().into_boxed_str()) + }; + properties.push(PropertyDeclaration::BorderTopColor(c)); + + let serialization = shorthand_properties_to_string(properties); + assert_eq!(serialization, "border-color: green red red; border-top: 1px solid green;"); + } +