Bug 1430817: Serialize <overflow-position> before other align bits. r?mats draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 24 Jan 2018 18:13:53 +0100
changeset 747861 e009270cd439c6d1ff86007cffae666104d500a5
parent 747732 617df9c9618b2e754029585556f3150766922d16
child 747862 e33105723cb17b21533073b5a199971ca5d7b2ac
push id97026
push userbmo:emilio@crisal.io
push dateFri, 26 Jan 2018 23:13:24 +0000
reviewersmats
bugs1430817
milestone60.0a1
Bug 1430817: Serialize <overflow-position> before other align bits. r?mats Otherwise the serialisation wouldn't roundtrip with the new syntax, which fixes the position of <overflow-position>. Also make Servo and Gecko agree on whether to serialize "unsafe". MozReview-Commit-ID: L3GSMk5pZ3F
layout/style/nsCSSValue.cpp
servo/components/style/values/specified/align.rs
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -1260,17 +1260,21 @@ nsCSSValue::AppendInsetToString(nsCSSPro
 /* static */ void
 nsCSSValue::AppendAlignJustifyValueToString(int32_t aValue, nsAString& aResult)
 {
   auto legacy = aValue & NS_STYLE_ALIGN_LEGACY;
   if (legacy) {
     aValue &= ~legacy;
     aResult.AppendLiteral("legacy ");
   }
+  // Don't serialize the 'unsafe' keyword; it's the default.
   auto overflowPos = aValue & (NS_STYLE_ALIGN_SAFE | NS_STYLE_ALIGN_UNSAFE);
+  if (MOZ_UNLIKELY(overflowPos == NS_STYLE_ALIGN_SAFE)) {
+    aResult.AppendLiteral("safe ");
+  }
   aValue &= ~overflowPos;
   MOZ_ASSERT(!(aValue & NS_STYLE_ALIGN_FLAG_BITS),
              "unknown bits in align/justify value");
   MOZ_ASSERT((aValue != NS_STYLE_ALIGN_AUTO &&
               aValue != NS_STYLE_ALIGN_NORMAL &&
               aValue != NS_STYLE_ALIGN_BASELINE &&
               aValue != NS_STYLE_ALIGN_LAST_BASELINE) ||
              (!legacy && !overflowPos),
@@ -1278,22 +1282,16 @@ nsCSSValue::AppendAlignJustifyValueToStr
   MOZ_ASSERT(legacy == 0 || overflowPos == 0,
              "'legacy' together with <overflow-position>");
   if (aValue == NS_STYLE_ALIGN_LAST_BASELINE) {
     aResult.AppendLiteral("last ");
     aValue = NS_STYLE_ALIGN_BASELINE;
   }
   const auto& kwtable(nsCSSProps::kAlignAllKeywords);
   AppendASCIItoUTF16(nsCSSProps::ValueToKeyword(aValue, kwtable), aResult);
-  // Don't serialize the 'unsafe' keyword; it's the default.
-  if (MOZ_UNLIKELY(overflowPos == NS_STYLE_ALIGN_SAFE)) {
-    aResult.Append(' ');
-    AppendASCIItoUTF16(nsCSSProps::ValueToKeyword(overflowPos, kwtable),
-                       aResult);
-  }
 }
 
 /**
  * Returns a re-ordered version of an csCSSValue::Array representing a shadow
  * item (including a drop-shadow() filter function) suitable for serialization.
  */
 static already_AddRefed<nsCSSValue::Array>
 GetReorderedShadowArrayForSerialization(const nsCSSValue::Array* aOriginalArray)
--- a/servo/components/style/values/specified/align.rs
+++ b/servo/components/style/values/specified/align.rs
@@ -70,17 +70,24 @@ bitflags! {
     }
 }
 
 impl ToCss for AlignFlags {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
     {
-        let s = match *self & !AlignFlags::FLAG_BITS {
+        match *self & AlignFlags::FLAG_BITS {
+            AlignFlags::LEGACY => dest.write_str("legacy ")?,
+            AlignFlags::SAFE => dest.write_str("safe ")?,
+            // Don't serialize "unsafe", since it's the default.
+            _ => {}
+        }
+
+        dest.write_str(match *self & !AlignFlags::FLAG_BITS {
             AlignFlags::AUTO => "auto",
             AlignFlags::NORMAL => "normal",
             AlignFlags::START => "start",
             AlignFlags::END => "end",
             AlignFlags::FLEX_START => "flex-start",
             AlignFlags::FLEX_END => "flex-end",
             AlignFlags::CENTER => "center",
             AlignFlags::LEFT => "left",
@@ -89,26 +96,17 @@ impl ToCss for AlignFlags {
             AlignFlags::LAST_BASELINE => "last baseline",
             AlignFlags::STRETCH => "stretch",
             AlignFlags::SELF_START => "self-start",
             AlignFlags::SELF_END => "self-end",
             AlignFlags::SPACE_BETWEEN => "space-between",
             AlignFlags::SPACE_AROUND => "space-around",
             AlignFlags::SPACE_EVENLY => "space-evenly",
             _ => unreachable!()
-        };
-        dest.write_str(s)?;
-
-        match *self & AlignFlags::FLAG_BITS {
-            AlignFlags::LEGACY => { dest.write_str(" legacy")?; }
-            AlignFlags::SAFE => { dest.write_str(" safe")?; }
-            AlignFlags::UNSAFE => { dest.write_str(" unsafe")?; }
-            _ => {}
-        }
-        Ok(())
+        })
     }
 }
 
 /// Mask for a single AlignFlags value.
 const ALIGN_ALL_BITS: u16 = structs::NS_STYLE_ALIGN_ALL_BITS as u16;
 /// Number of bits to shift a fallback alignment.
 const ALIGN_ALL_SHIFT: u32 = structs::NS_STYLE_ALIGN_ALL_SHIFT;