Skip to content

Commit

Permalink
Fix overflow shorthand serialization.
Browse files Browse the repository at this point in the history
  • Loading branch information
canova committed Dec 31, 2016
1 parent bd67163 commit c351092
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 10 deletions.
15 changes: 10 additions & 5 deletions components/style/properties/declaration_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl PropertyDeclarationBlock {
// so we treat this as a normal-importance property
let importance = Importance::Normal;
let appendable_value = shorthand.get_shorthand_appendable_value(list).unwrap();
append_declaration_value(dest, appendable_value, importance)
append_declaration_value(dest, appendable_value, importance, false)
}
Err(longhand_or_custom) => {
if let Some(&(ref value, _importance)) = self.get(longhand_or_custom) {
Expand Down Expand Up @@ -377,7 +377,8 @@ fn handle_first_serialization<W>(dest: &mut W, is_first_serialization: &mut bool
pub fn append_declaration_value<'a, W, I>
(dest: &mut W,
appendable_value: AppendableValue<'a, I>,
importance: Importance)
importance: Importance,
is_overflow_with_name: bool)
-> fmt::Result
where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
match appendable_value {
Expand All @@ -388,7 +389,11 @@ pub fn append_declaration_value<'a, W, I>
try!(decl.to_css(dest));
},
AppendableValue::DeclarationsForShorthand(shorthand, decls) => {
try!(shorthand.longhands_to_css(decls, dest));
if is_overflow_with_name {
try!(shorthand.overflow_longhands_to_css(decls, dest));
} else {
try!(shorthand.longhands_to_css(decls, dest));
}
}
}

Expand All @@ -414,7 +419,7 @@ pub fn append_serialization<'a, W, I, N>(dest: &mut W,
// Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
// values, they no longer use the shared property name "overflow" and must be handled differently
if shorthands::is_overflow_shorthand(&appendable_value) {
return append_declaration_value(dest, appendable_value, importance);
return append_declaration_value(dest, appendable_value, importance, true);
}

try!(property_name.to_css(dest));
Expand All @@ -434,7 +439,7 @@ pub fn append_serialization<'a, W, I, N>(dest: &mut W,
&AppendableValue::DeclarationsForShorthand(..) => try!(write!(dest, " "))
}

try!(append_declaration_value(dest, appendable_value, importance));
try!(append_declaration_value(dest, appendable_value, importance, false));
write!(dest, ";")
}

Expand Down
15 changes: 15 additions & 0 deletions components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,21 @@ impl ShorthandId {
}
}

// Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
// values, they no longer use the shared property name "overflow".
pub fn overflow_longhands_to_css<'a, W, I>(&self, declarations: I, dest: &mut W) -> fmt::Result
where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
match *self {
ShorthandId::Overflow => {
match shorthands::overflow::LonghandsToSerialize::from_iter(declarations) {
Ok(longhands) => longhands.to_css_declared_with_name(dest),
Err(_) => Err(fmt::Error)
}
},
_ => Err(fmt::Error)
}
}

/// Serializes the possible shorthand name with value to input buffer given
/// a list of longhand declarations.
///
Expand Down
29 changes: 25 additions & 4 deletions components/style/properties/shorthand/box.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@
})
}


// Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
// values, they no longer use the shared property name "overflow".
// Other shorthands do not include their name in the to_css method
impl<'a> LonghandsToSerialize<'a> {
fn to_css_declared<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
let x_and_y_equal = match (self.overflow_x, self.overflow_y) {
Expand All @@ -32,6 +28,31 @@
_ => false
};

if x_and_y_equal {
try!(self.overflow_x.to_css(dest));
}
Ok(())
}

// Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
// values, they no longer use the shared property name "overflow".
// Other shorthands do not include their name in the to_css method
pub fn to_css_declared_with_name<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
let x_and_y_equal = match (self.overflow_x, self.overflow_y) {
(&DeclaredValue::Value(ref x_value), &DeclaredValue::Value(ref y_container)) => {
*x_value == y_container.0
},
(_, &DeclaredValue::WithVariables { .. }) |
(&DeclaredValue::WithVariables { .. }, _) => {
// We don't serialize shorthands with variables
return dest.write_str("");
},
(&DeclaredValue::Initial, &DeclaredValue::Initial) => true,
(&DeclaredValue::Inherit, &DeclaredValue::Inherit) => true,
(&DeclaredValue::Unset, &DeclaredValue::Unset) => true,
_ => false
};

if x_and_y_equal {
try!(write!(dest, "overflow"));
try!(write!(dest, ": "));
Expand Down
11 changes: 10 additions & 1 deletion tests/wpt/metadata/MANIFEST.json
Original file line number Diff line number Diff line change
Expand Up @@ -45845,7 +45845,16 @@
"local_changes": {
"deleted": [],
"deleted_reftests": {},
"items": {},
"items": {
"testharness": {
"cssom/overflow-serialization.html": [
{
"path": "cssom/overflow-serialization.html",
"url": "/cssom/overflow-serialization.html"
}
]
}
},
"reftest_nodes": {}
},
"reftest_nodes": {
Expand Down
48 changes: 48 additions & 0 deletions tests/wpt/web-platform-tests/cssom/overflow-serialization.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>CSSOM - Overlow property has different serialization than other shorthands.</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
div { overflow: inherit; }
div { overflow: hidden; }
div { overflow-x: initial; overflow-y: initial; }
div { overflow-x: scroll; overflow-y: scroll; }
div { overflow-x: inherit; overflow-y: unset; }
</style>

<script>
test(function () {
var styleSheet = document.styleSheets[0];

assert_equals(styleSheet.cssRules[0].style.cssText, "overflow: inherit;", "Single value overflow with CSS-wide keyword should serialize correctly.");
assert_equals(styleSheet.cssRules[1].style.cssText, "overflow: hidden;", "Single value overflow with non-CSS-wide keyword should serialize correctly.");
assert_equals(styleSheet.cssRules[2].style.cssText, "overflow: initial;", "Overflow-x/y longhands with same CSS-wide keyword should serialize correctly.");
assert_equals(styleSheet.cssRules[3].style.cssText, "overflow: scroll;", "Overflow-x/y longhands with same non-CSS-wide keyword should serialize correctly.");
assert_equals(styleSheet.cssRules[4].style.cssText, "overflow-x: inherit; overflow-y: unset;", "Overflow-x/y longhands with different keywords should serialize correctly.");

var div = document.createElement('div');
div.style.overflow = "inherit";
assert_equals(div.style.overflow, "inherit", "Single value overflow with CSS-wide keyword should serialize correctly.");

div.style.overflow = "hidden";
assert_equals(div.style.overflow, "hidden", "Single value overflow with non-CSS-wide keyword should serialize correctly.");

div.style.overflow = "";
div.style.overflowX = "initial";
div.style.overflowY = "initial";
assert_equals(div.style.overflow, "initial", "Overflow-x/y longhands with same CSS-wide keyword should serialize correctly.");

div.style.overflowX = "scroll";
div.style.overflowY = "scroll";
assert_equals(div.style.overflow, "scroll", "Overflow-x/y longhands with same non-CSS-wide keyword should serialize correctly.");

div.style.overflowX = "inherit";
div.style.overflowY = "unset";
assert_equals(div.style.overflow, "", "Overflow-x/y longhands with different keywords shouldn't serialize.");
});
</script>
</head>
</html>

0 comments on commit c351092

Please sign in to comment.