Bug 775618 - Implement page-break-{before,after} as legacy shorthands for {before,after}. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 30 Nov 2018 05:35:47 +0000
changeset 448933 b4c0c0fecdba68979d41609d3978fe596078d0cd
parent 448932 f97f45821194fd28be1282d3a854ed129498d187
child 448934 7cd2172b993f1c7c876f415c11549bd319aebbf3
push id35129
push usernerli@mozilla.com
push dateFri, 30 Nov 2018 09:34:14 +0000
treeherdermozilla-central@c5b713000513 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs775618, 906336, 191803
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 775618 - Implement page-break-{before,after} as legacy shorthands for {before,after}. r=heycam This is all the style-system work needed for this. This implements the concept of legacy shorthands, teaches tests to understand it, and adds a few more tests for these properties in particular. The WPT even caught a few WebKit / Blink bugs: https://bugs.chromium.org/p/chromium/issues/detail?id=906336 https://bugs.webkit.org/show_bug.cgi?id=191803 This doesn't change the layout behavior for page-break-before: always, since it'd stop breaking in multicol and such. Similarly, break-before / break-after: column and page still behave the same, I'll file followups for those given comment 22. Differential Revision: https://phabricator.services.mozilla.com/D12211
devtools/server/actors/animation-type-longhand.js
devtools/shared/css/generated/properties-db.js
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
layout/style/test/property_database.js
layout/style/test/test_property_database.html
layout/style/test/test_value_storage.html
servo/components/style/properties/longhands/box.mako.rs
servo/components/style/properties/shorthands/box.mako.rs
servo/components/style/values/specified/box.rs
testing/web-platform/meta/css/css-break/inheritance.html.ini
testing/web-platform/tests/css/css-break/page-break-legacy-shorthands.html
--- a/devtools/server/actors/animation-type-longhand.js
+++ b/devtools/server/actors/animation-type-longhand.js
@@ -109,19 +109,19 @@ exports.ANIMATION_TYPE_FOR_LONGHANDS = [
     "outline-style",
     "overflow-clip-box-block",
     "overflow-clip-box-inline",
     "overflow-wrap",
     "overflow-x",
     "overflow-y",
     "overscroll-behavior-x",
     "overscroll-behavior-y",
-    "page-break-after",
-    "page-break-before",
-    "page-break-inside",
+    "break-after",
+    "break-before",
+    "break-inside",
     "paint-order",
     "pointer-events",
     "position",
     "quotes",
     "resize",
     "ruby-align",
     "ruby-position",
     "scroll-behavior",
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@@ -2935,19 +2935,19 @@ exports.CSS_PROPERTIES = {
       "translate",
       "offset-path",
       "scroll-behavior",
       "scroll-snap-type-x",
       "scroll-snap-type-y",
       "overscroll-behavior-x",
       "overscroll-behavior-y",
       "isolation",
-      "page-break-after",
-      "page-break-before",
-      "page-break-inside",
+      "break-after",
+      "break-before",
+      "break-inside",
       "resize",
       "perspective",
       "perspective-origin",
       "backface-visibility",
       "transform-box",
       "transform-style",
       "transform-origin",
       "contain",
@@ -4770,16 +4770,66 @@ exports.CSS_PROPERTIES = {
     "values": [
       "border-box",
       "content-box",
       "inherit",
       "initial",
       "unset"
     ]
   },
+  "break-after": {
+    "isInherited": false,
+    "subproperties": [
+      "break-after"
+    ],
+    "supports": [],
+    "values": [
+      "always",
+      "auto",
+      "avoid",
+      "inherit",
+      "initial",
+      "left",
+      "page",
+      "right",
+      "unset"
+    ]
+  },
+  "break-before": {
+    "isInherited": false,
+    "subproperties": [
+      "break-before"
+    ],
+    "supports": [],
+    "values": [
+      "always",
+      "auto",
+      "avoid",
+      "inherit",
+      "initial",
+      "left",
+      "page",
+      "right",
+      "unset"
+    ]
+  },
+  "break-inside": {
+    "isInherited": false,
+    "subproperties": [
+      "break-inside"
+    ],
+    "supports": [],
+    "values": [
+      "auto",
+      "avoid",
+      "inherit",
+      "initial",
+      "unset"
+    ]
+  },
   "caption-side": {
     "isInherited": true,
     "subproperties": [
       "caption-side"
     ],
     "supports": [],
     "values": [
       "bottom",
@@ -7767,51 +7817,53 @@ exports.CSS_PROPERTIES = {
       "inherit",
       "initial",
       "unset"
     ]
   },
   "page-break-after": {
     "isInherited": false,
     "subproperties": [
-      "page-break-after"
+      "break-after"
     ],
     "supports": [],
     "values": [
       "always",
       "auto",
       "avoid",
       "inherit",
       "initial",
       "left",
+      "page",
       "right",
       "unset"
     ]
   },
   "page-break-before": {
     "isInherited": false,
     "subproperties": [
-      "page-break-before"
+      "break-before"
     ],
     "supports": [],
     "values": [
       "always",
       "auto",
       "avoid",
       "inherit",
       "initial",
       "left",
+      "page",
       "right",
       "unset"
     ]
   },
   "page-break-inside": {
     "isInherited": false,
     "subproperties": [
-      "page-break-inside"
+      "break-inside"
     ],
     "supports": [],
     "values": [
       "auto",
       "avoid",
       "inherit",
       "initial",
       "unset"
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3380,18 +3380,18 @@ nsStyleDisplay::nsStyleDisplay(const nsP
   , mOriginalDisplay(StyleDisplay::Inline)
   , mContain(NS_STYLE_CONTAIN_NONE)
   , mAppearance(StyleAppearance::None)
   , mPosition(NS_STYLE_POSITION_STATIC)
   , mFloat(StyleFloat::None)
   , mOriginalFloat(StyleFloat::None)
   , mBreakType(StyleClear::None)
   , mBreakInside(StyleBreakWithin::Auto)
-  , mPageBreakBefore(StyleBreakBetween::Auto)
-  , mPageBreakAfter(StyleBreakBetween::Auto)
+  , mBreakBefore(StyleBreakBetween::Auto)
+  , mBreakAfter(StyleBreakBetween::Auto)
   , mOverflowX(NS_STYLE_OVERFLOW_VISIBLE)
   , mOverflowY(NS_STYLE_OVERFLOW_VISIBLE)
   , mOverflowClipBoxBlock(NS_STYLE_OVERFLOW_CLIP_BOX_PADDING_BOX)
   , mOverflowClipBoxInline(NS_STYLE_OVERFLOW_CLIP_BOX_PADDING_BOX)
   , mResize(NS_STYLE_RESIZE_NONE)
   , mOrient(StyleOrient::Inline)
   , mIsolation(NS_STYLE_ISOLATION_AUTO)
   , mTopLayer(NS_STYLE_TOP_LAYER_NONE)
@@ -3445,18 +3445,18 @@ nsStyleDisplay::nsStyleDisplay(const nsS
   , mOriginalDisplay(aSource.mOriginalDisplay)
   , mContain(aSource.mContain)
   , mAppearance(aSource.mAppearance)
   , mPosition(aSource.mPosition)
   , mFloat(aSource.mFloat)
   , mOriginalFloat(aSource.mOriginalFloat)
   , mBreakType(aSource.mBreakType)
   , mBreakInside(aSource.mBreakInside)
-  , mPageBreakBefore(aSource.mPageBreakBefore)
-  , mPageBreakAfter(aSource.mPageBreakAfter)
+  , mBreakBefore(aSource.mBreakBefore)
+  , mBreakAfter(aSource.mBreakAfter)
   , mOverflowX(aSource.mOverflowX)
   , mOverflowY(aSource.mOverflowY)
   , mOverflowClipBoxBlock(aSource.mOverflowClipBoxBlock)
   , mOverflowClipBoxInline(aSource.mOverflowClipBoxInline)
   , mResize(aSource.mResize)
   , mOrient(aSource.mOrient)
   , mIsolation(aSource.mIsolation)
   , mTopLayer(aSource.mTopLayer)
@@ -3685,18 +3685,18 @@ nsStyleDisplay::CalcDifference(const nsS
 
   // XXX the following is conservative, for now: changing float breaking shouldn't
   // necessarily require a repaint, reflow should suffice.
   //
   // FIXME(emilio): We definitely change the frame tree in nsCSSFrameConstructor
   // based on break-before / break-after... Shouldn't that reframe?
   if (mBreakType != aNewData.mBreakType
       || mBreakInside != aNewData.mBreakInside
-      || mPageBreakBefore != aNewData.mPageBreakBefore
-      || mPageBreakAfter != aNewData.mPageBreakAfter
+      || mBreakBefore != aNewData.mBreakBefore
+      || mBreakAfter != aNewData.mBreakAfter
       || mAppearance != aNewData.mAppearance
       || mOrient != aNewData.mOrient
       || mOverflowClipBoxBlock != aNewData.mOverflowClipBoxBlock
       || mOverflowClipBoxInline != aNewData.mOverflowClipBoxInline) {
     hint |= nsChangeHint_AllReflowHints |
             nsChangeHint_RepaintFrame;
   }
 
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -2010,18 +2010,18 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
   uint8_t mPosition;            // NS_STYLE_POSITION_*
 
   mozilla::StyleFloat mFloat;
   // Save mFloat for position:absolute/fixed; otherwise equal to mFloat.
   mozilla::StyleFloat mOriginalFloat;
 
   mozilla::StyleClear mBreakType;
   mozilla::StyleBreakWithin mBreakInside;
-  mozilla::StyleBreakBetween mPageBreakBefore;
-  mozilla::StyleBreakBetween mPageBreakAfter;
+  mozilla::StyleBreakBetween mBreakBefore;
+  mozilla::StyleBreakBetween mBreakAfter;
   uint8_t mOverflowX;           // NS_STYLE_OVERFLOW_*
   uint8_t mOverflowY;           // NS_STYLE_OVERFLOW_*
   uint8_t mOverflowClipBoxBlock;     // NS_STYLE_OVERFLOW_CLIP_BOX_*
   uint8_t mOverflowClipBoxInline;    // NS_STYLE_OVERFLOW_CLIP_BOX_*
   uint8_t mResize;              // NS_STYLE_RESIZE_*
   mozilla::StyleOrient mOrient;
   uint8_t mIsolation;           // NS_STYLE_ISOLATION_*
   uint8_t mTopLayer;            // NS_STYLE_TOP_LAYER_*
@@ -2319,44 +2319,43 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
   bool HasPerspectiveStyle() const {
     return mChildPerspective.GetUnit() == eStyleUnit_Coord;
   }
 
   bool BackfaceIsHidden() const {
     return mBackfaceVisibility == NS_STYLE_BACKFACE_VISIBILITY_HIDDEN;
   }
 
-  // FIXME(emilio): This looks slightly fishy, this had a comment from karnaze
-  // with "Temp fix for bug 24000" on it... Oh well.
+  // FIXME(emilio): This should be more fine-grained on each caller to
+  // BreakBefore() / BreakAfter().
   static bool ShouldBreak(mozilla::StyleBreakBetween aBreak)
   {
     switch (aBreak) {
-      // "A conforming user agent may interpret the values 'left' and 'right' as
-      // 'always'." - CSS2.1, section 13.3.1
       case mozilla::StyleBreakBetween::Left:
       case mozilla::StyleBreakBetween::Right:
+      case mozilla::StyleBreakBetween::Page:
       case mozilla::StyleBreakBetween::Always:
         return true;
       case mozilla::StyleBreakBetween::Auto:
       case mozilla::StyleBreakBetween::Avoid:
         return false;
       default:
         MOZ_ASSERT_UNREACHABLE("Unknown break kind");
         return false;
     }
   }
 
   bool BreakBefore() const
   {
-    return ShouldBreak(mPageBreakBefore);
+    return ShouldBreak(mBreakBefore);
   }
 
   bool BreakAfter() const
   {
-    return ShouldBreak(mPageBreakAfter);
+    return ShouldBreak(mBreakAfter);
   }
 
   // These are defined in nsStyleStructInlines.h.
 
   // The aContextFrame argument on each of these is the frame this
   // style struct is for.  If the frame is for SVG text, the return
   // value will be massaged to be something that makes sense for
   // SVG text.
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -31,16 +31,21 @@ const CSS_TYPE_LONGHAND = 0;
 
 // True shorthand properties.
 const CSS_TYPE_TRUE_SHORTHAND = 1;
 
 // Properties that we handle as shorthands but were longhands either in
 // the current spec or earlier versions of the spec.
 const CSS_TYPE_SHORTHAND_AND_LONGHAND = 2;
 
+// Legacy shorthand properties, that behave mostly like an alias
+// (CSS_TYPE_SHORTHAND_AND_LONGHAND) but not quite because their syntax may not
+// match, plus they shouldn't serialize in cssText.
+const CSS_TYPE_LEGACY_SHORTHAND = 3;
+
 // Each property has the following fields:
 //   domProp: The name of the relevant member of nsIDOM[NS]CSS2Properties
 //   inherited: Whether the property is inherited by default (stated as
 //     yes or no in the property header in all CSS specs)
 //   type: see above
 //   alias_for: optional, indicates that the property is an alias for
 //     some other property that is the preferred serialization.  (Type
 //     must not be CSS_TYPE_LONGHAND.)
@@ -4294,28 +4299,54 @@ var gCSSProperties = {
       "calc(3*25px + 50%)",
     ],
     invalid_values: [ ],
     quirks_values: { "5": "5px" },
   },
   "page-break-after": {
     domProp: "pageBreakAfter",
     inherited: false,
-    type: CSS_TYPE_LONGHAND,
+    type: CSS_TYPE_LEGACY_SHORTHAND,
+    alias_for: "break-after",
+    subproperties: [ "break-after" ],
     initial_values: [ "auto" ],
     other_values: [ "always", "avoid", "left", "right" ],
-    invalid_values: []
+    legacy_mapping: {
+      always: "page",
+    },
+    invalid_values: [ "page", "column" ]
   },
   "page-break-before": {
     domProp: "pageBreakBefore",
     inherited: false,
+    type: CSS_TYPE_LEGACY_SHORTHAND,
+    alias_for: "break-before",
+    subproperties: [ "break-before" ],
+    initial_values: [ "auto" ],
+    other_values: [ "always", "avoid", "left", "right" ],
+    legacy_mapping: {
+      always: "page",
+    },
+    invalid_values: [ "page", "column" ]
+  },
+  "break-after": {
+    domProp: "breakAfter",
+    inherited: false,
     type: CSS_TYPE_LONGHAND,
     initial_values: [ "auto" ],
-    other_values: [ "always", "avoid", "left", "right" ],
-    invalid_values: []
+    other_values: [ "always", "page", "avoid", "left", "right" ],
+    invalid_values: [ ]
+  },
+  "break-before": {
+    domProp: "breakBefore",
+    inherited: false,
+    type: CSS_TYPE_LONGHAND,
+    initial_values: [ "auto" ],
+    other_values: [ "always", "page", "avoid", "left", "right" ],
+    invalid_values: [ ]
   },
   "break-inside": {
     domProp: "breakInside",
     inherited: false,
     type: CSS_TYPE_LONGHAND,
     initial_values: [ "auto" ],
     other_values: [ "avoid" ],
     invalid_values: [ "left", "right", "always" ]
--- a/layout/style/test/test_property_database.html
+++ b/layout/style/test/test_property_database.html
@@ -55,18 +55,19 @@ for (var idx in gShorthandProperties) {
     // "all" isn't listed in property_database.js.
     continue;
   }
   var present = prop.name in gCSSProperties;
   ok(present,
      "'" + prop.name + "' listed in gCSSProperties");
   if (present) {
     ok(gCSSProperties[prop.name].type == CSS_TYPE_TRUE_SHORTHAND ||
+       gCSSProperties[prop.name].type == CSS_TYPE_LEGACY_SHORTHAND ||
        gCSSProperties[prop.name].type == CSS_TYPE_SHORTHAND_AND_LONGHAND,
-       "'" + prop.name + "' listed as CSS_TYPE_TRUE_SHORTHAND or CSS_TYPE_SHORTHAND_AND_LONGHAND");
+       "'" + prop.name + "' listed as CSS_TYPE_TRUE_SHORTHAND, CSS_TYPE_LEGACY_SHORTHAND, or CSS_TYPE_SHORTHAND_AND_LONGHAND");
     ok(gCSSProperties[prop.name].domProp == prop.prop,
        "'" + prop.name + "' listed with correct DOM property name");
   }
 }
 for (var idx in gShorthandPropertiesLikeLonghand) {
   var prop = gShorthandPropertiesLikeLonghand[idx];
   if (prop.pref && !IsCSSPropertyPrefEnabled(prop.pref)) {
     continue;
--- a/layout/style/test/test_value_storage.html
+++ b/layout/style/test/test_value_storage.html
@@ -177,32 +177,39 @@ function test_property(property)
 
     SimpleTest.isnot(step1val, "", "setting '" + value + "' on '" + property + "'");
     if ("subproperties" in info &&
         // System font doesn't produce meaningful value for subproperties.
         !is_system_font)
       for (idx in info.subproperties) {
         var subprop = info.subproperties[idx];
         if (value_has_variable_reference &&
-            (!info.alias_for || info.type == CSS_TYPE_TRUE_SHORTHAND)) {
+            (!info.alias_for || info.type == CSS_TYPE_TRUE_SHORTHAND ||
+            info.type == CSS_TYPE_LEGACY_SHORTHAND)) {
           is(gDeclaration.getPropertyValue(subprop), "",
              "setting '" + value + "' on '" + property + "' (for '" + subprop + "')");
           test_other_shorthands_empty(value, subprop);
         } else {
           isnot(gDeclaration.getPropertyValue(subprop), "",
                 "setting '" + value + "' on '" + property + "' (for '" + subprop + "')");
         }
       }
 
     // We don't care particularly about the whitespace or the placement of
     // semicolons, but for simplicity we'll test the current behavior.
     var expected_serialization = "";
     if (step1val != "") {
       if ("alias_for" in info) {
-        expected_serialization = info.alias_for + colon + step1val + ";";
+        let value = info.legacy_mapping && info.legacy_mapping[step1val]
+          ? info.legacy_mapping[step1val] : step1val;
+        // FIXME(emilio): This is a bit unfortunate:
+        // https://github.com/w3c/csswg-drafts/issues/3332
+        if (info.type == CSS_TYPE_LEGACY_SHORTHAND && value_has_variable_reference)
+          value = "";
+        expected_serialization = info.alias_for + colon + value + ";";
       } else {
         expected_serialization = property + colon + step1val + ";";
       }
     }
     is(step1ser, expected_serialization,
        "serialization should match property value");
 
     gDeclaration.removeProperty(property);
@@ -216,18 +223,16 @@ function test_property(property)
          "serialize+parse should be identity transform for '" +
          property + ": " + value + "'");
     }
 
     if ("subproperties" in info &&
         // Using setProperty over subproperties is not sufficient for
         // system fonts, since the shorthand does more than its parts.
         !is_system_font &&
-        // Likewise for special compatibility values of transform
-        (property != "-moz-transform" || !value.match(/^matrix.*(px|em|%)/)) &&
         !value_has_variable_reference) {
       gDeclaration.removeProperty(property);
       for (idx in info.subproperties) {
         var subprop = info.subproperties[idx];
         gDeclaration.setProperty(subprop, step1vals[idx], "");
       }
 
       // Now that all the subprops are set, check their values.  Note that we
@@ -239,18 +244,19 @@ function test_property(property)
            "serialize(" + subprop + ")+parse should be the identity " +
            "transform for '" + property + ": " + value + "'");
       }
       is(gDeclaration.getPropertyValue(property), step1val,
          "parse+split+serialize should be idempotent for '" +
          property + colon + value + "'");
     }
 
+    // FIXME(emilio): Why is mask special?
     if (info.type != CSS_TYPE_TRUE_SHORTHAND &&
-          property != "mask") {
+        property != "mask") {
       gDeclaration.removeProperty(property);
       gDeclaration.setProperty(property, step1comp, "");
       var func = xfail_compute(property, value) ? todo_is : is;
       func(gComputedStyle.getPropertyValue(property), step1comp,
            "parse+compute+serialize should be idempotent for '" +
            property + ": " + value + "'");
     }
     if ("subproperties" in info && !is_system_font) {
--- a/servo/components/style/properties/longhands/box.mako.rs
+++ b/servo/components/style/properties/longhands/box.mako.rs
@@ -433,32 +433,32 @@
     products="gecko",
     gecko_pref="layout.css.isolation.enabled",
     spec="https://drafts.fxtf.org/compositing/#isolation",
     flags="CREATES_STACKING_CONTEXT",
     animation_value_type="discrete",
 )}
 
 ${helpers.predefined_type(
-    "page-break-after",
+    "break-after",
     "BreakBetween",
     "computed::BreakBetween::Auto",
     needs_context=False,
     products="gecko",
-    spec="https://drafts.csswg.org/css2/page.html#propdef-page-break-after",
+    spec="https://drafts.csswg.org/css-break/#propdef-break-after",
     animation_value_type="discrete",
 )}
 
 ${helpers.predefined_type(
-    "page-break-before",
+    "break-before",
     "BreakBetween",
     "computed::BreakBetween::Auto",
     needs_context=False,
     products="gecko",
-    spec="https://drafts.csswg.org/css2/page.html#propdef-page-break-before",
+    spec="https://drafts.csswg.org/css-break/#propdef-break-before",
     animation_value_type="discrete",
 )}
 
 ${helpers.predefined_type(
     "break-inside",
     "BreakWithin",
     "computed::BreakWithin::Auto",
     needs_context=False,
--- a/servo/components/style/properties/shorthands/box.mako.rs
+++ b/servo/components/style/properties/shorthands/box.mako.rs
@@ -442,8 +442,54 @@ macro_rules! try_parse_one {
             if self.overscroll_behavior_y != self.overscroll_behavior_x {
                 dest.write_str(" ")?;
                 self.overscroll_behavior_y.to_css(dest)?;
             }
             Ok(())
         }
     }
 </%helpers:shorthand>
+
+<%helpers:shorthand
+    name="page-break-before"
+    flags="SHORTHAND_IN_GETCS IS_LEGACY_SHORTHAND"
+    sub_properties="break-before"
+    spec="https://drafts.csswg.org/css2/page.html#propdef-page-break-before"
+>
+    pub fn parse_value<'i>(
+        _: &ParserContext,
+        input: &mut Parser<'i, '_>,
+    ) -> Result<Longhands, ParseError<'i>> {
+        use crate::values::specified::box_::BreakBetween;
+        Ok(expanded! {
+            break_before: BreakBetween::parse_legacy(input)?,
+        })
+    }
+
+    impl<'a> ToCss for LonghandsToSerialize<'a> {
+        fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
+            self.break_before.to_css_legacy(dest)
+        }
+    }
+</%helpers:shorthand>
+
+<%helpers:shorthand
+    name="page-break-after"
+    flags="SHORTHAND_IN_GETCS IS_LEGACY_SHORTHAND"
+    sub_properties="break-after"
+    spec="https://drafts.csswg.org/css2/page.html#propdef-page-break-after"
+>
+    pub fn parse_value<'i>(
+        _: &ParserContext,
+        input: &mut Parser<'i, '_>,
+    ) -> Result<Longhands, ParseError<'i>> {
+        use crate::values::specified::box_::BreakBetween;
+        Ok(expanded! {
+            break_after: BreakBetween::parse_legacy(input)?,
+        })
+    }
+
+    impl<'a> ToCss for LonghandsToSerialize<'a> {
+        fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
+            self.break_after.to_css_legacy(dest)
+        }
+    }
+</%helpers:shorthand>
--- a/servo/components/style/values/specified/box.rs
+++ b/servo/components/style/values/specified/box.rs
@@ -1281,23 +1281,70 @@ pub enum Appearance {
     Parse,
     PartialEq,
     SpecifiedValueInfo,
     ToCss,
     ToComputedValue,
 )]
 #[repr(u8)]
 pub enum BreakBetween {
+    Always,
     Auto,
-    Always,
+    Page,
     Avoid,
     Left,
     Right,
 }
 
+impl BreakBetween {
+    /// Parse a legacy break-between value for `page-break-*`.
+    ///
+    /// See https://drafts.csswg.org/css-break/#page-break-properties.
+    #[inline]
+    pub fn parse_legacy<'i>(
+        input: &mut Parser<'i, '_>,
+    ) -> Result<Self, ParseError<'i>> {
+        let location = input.current_source_location();
+        let ident = input.expect_ident()?;
+        let break_value = match BreakBetween::from_ident(ident) {
+            Ok(v) => v,
+            Err(()) => return Err(location.new_custom_error(
+                SelectorParseErrorKind::UnexpectedIdent(ident.clone())
+            )),
+        };
+        match break_value {
+            BreakBetween::Always => Ok(BreakBetween::Page),
+            BreakBetween::Auto |
+            BreakBetween::Avoid |
+            BreakBetween::Left |
+            BreakBetween::Right => Ok(break_value),
+            BreakBetween::Page => {
+                Err(location.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(ident.clone())))
+            }
+        }
+    }
+
+    /// Serialize a legacy break-between value for `page-break-*`.
+    ///
+    /// See https://drafts.csswg.org/css-break/#page-break-properties.
+    pub fn to_css_legacy<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        match *self {
+            BreakBetween::Auto |
+            BreakBetween::Avoid |
+            BreakBetween::Left |
+            BreakBetween::Right => self.to_css(dest),
+            BreakBetween::Page => dest.write_str("always"),
+            BreakBetween::Always => Ok(()),
+        }
+    }
+}
+
 /// A kind of break within a box.
 ///
 /// https://drafts.csswg.org/css-break/#break-within
 #[allow(missing_docs)]
 #[derive(
     Clone,
     Copy,
     Debug,
--- a/testing/web-platform/meta/css/css-break/inheritance.html.ini
+++ b/testing/web-platform/meta/css/css-break/inheritance.html.ini
@@ -1,4 +1,4 @@
 [inheritance.html]
-  [Inheritance of CSS Fragmentation properties]
+  [Property break-after does not inherit]
     expected: FAIL
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-break/page-break-legacy-shorthands.html
@@ -0,0 +1,75 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Test for the page-break-* legacy shorthands.</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<link rel="help" href="https://drafts.csswg.org/css-cascade-4/#legacy-shorthand">
+<link rel="help" href="https://drafts.csswg.org/css-break/#page-break-properties">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div></div>
+<script>
+const NEW_VALUES = ["page", "column"].filter(v => CSS.supports("break-before", v));
+const LEGACY_VALUES = ["always", "auto", "left", "right", "avoid"];
+const LEGACY_MAPPING = { "always": "page" };
+const REVERSE_LEGACY_MAPPING = { "page": "always" };
+const div = document.querySelector("div");
+const cs = getComputedStyle(div);
+
+test(function() {
+  for (const property of ["break-before", "break-after"]) {
+    for (const val of LEGACY_VALUES) {
+      const mapped_value = LEGACY_MAPPING[val] || val;
+
+      div.style["page-" + property] = val;
+
+      assert_equals(div.style["page-" + property], val);
+      assert_equals(div.style[property], mapped_value);
+      assert_equals(cs.getPropertyValue("page-" + property), val);
+      assert_equals(cs.getPropertyValue(property), mapped_value);
+      assert_not_equals(div.style.cssText.indexOf(property + ": " + mapped_value + ";"), -1);
+      assert_equals(div.style.cssText.indexOf("page-" + property), -1,
+                    "Legacy shorthands don't appear in cssText");
+    }
+  }
+}, "Legacy values of the shorthands work as expected")
+
+test(function() {
+  for (const property of ["break-before", "break-after"]) {
+    for (const val of NEW_VALUES) {
+      const mapped_value = REVERSE_LEGACY_MAPPING[val] || "";
+
+      div.style[property] = val;
+
+      assert_equals(div.style[property], val);
+      assert_equals(div.style["page-" + property], mapped_value);
+      assert_equals(cs.getPropertyValue("page-" + property), mapped_value);
+      assert_equals(cs.getPropertyValue(property), val);
+    }
+  }
+}, "New values work on the new longhands, but serialize to the empty string in the legacy shorthands");
+
+test(function() {
+  for (const property of ["break-before", "break-after"]) {
+    for (const val of NEW_VALUES) {
+      div.style["page-" + property] = "";
+      div.style["page-" + property] = val;
+      assert_equals(div.style["page-" + property], "");
+      assert_equals(div.style[property], "");
+      assert_equals(cs.getPropertyValue("page-" + property), "auto");
+      assert_equals(cs.getPropertyValue(property), "auto");
+    }
+  }
+}, "New values of the break longhands don't work on legacy shorthands");
+
+// See https://github.com/w3c/csswg-drafts/issues/3332
+test(function() {
+  for (const property of ["break-before", "break-after"]) {
+    div.style["page-" + property] = "var(--a)";
+
+    assert_equals(div.style["page-" + property], "var(--a)");
+    assert_equals(div.style[property], "");
+    assert_equals(div.style.cssText.indexOf("page-" + property), -1);
+  }
+}, "Legacy shorthands really never appear on cssText, even when there are variable references");
+</script>