Bug 1722945 - Support break-inside: avoid-{page,column}. r=TYLin

break-before/after: page|column seem harder because you need to deal
with nested breaks, I think, but this should be straight-forward.

Differential Revision: https://phabricator.services.mozilla.com/D121206

devtools/shared/css/generated/properties-db.js

@@ -5242,6 +5242,8 @@ exports.CSS_PROPERTIES = {
     "values": [
       "auto",
       "avoid",
+      "avoid-column",
+      "avoid-page",
       "inherit",
       "initial",
       "revert",
@@ -8950,6 +8952,8 @@ exports.CSS_PROPERTIES = {
     "values": [
       "auto",
       "avoid",
+      "avoid-column",
+      "avoid-page",
       "inherit",
       "initial",
       "revert",

layout/generic/ReflowInput.cpp

@@ -162,6 +162,7 @@ ReflowInput::ReflowInput(nsPresContext* aPresContext,
       mLineLayout(mFrame->IsFrameOfType(nsIFrame::eLineParticipant)
                       ? aParentReflowInput.mLineLayout
                       : nullptr),
+      mBreakType(aParentReflowInput.mBreakType),
       mPercentBSizeObserver(
           (aParentReflowInput.mPercentBSizeObserver &&
            aParentReflowInput.mPercentBSizeObserver->NeedsToObserve(*this))

layout/generic/ReflowInput.h

@@ -411,6 +411,13 @@ struct ReflowInput : public SizeComputationInput {
   const nsStylePadding* mStylePadding = nullptr;
   const nsStyleText* mStyleText = nullptr;
 
+  enum class BreakType : uint8_t {
+    Auto,
+    Column,
+    Page,
+  };
+  BreakType mBreakType = BreakType::Auto;
+
   // a frame (e.g. nsTableCellFrame) which may need to generate a special
   // reflow for percent bsize calculations
   nsIPercentBSizeObserver* mPercentBSizeObserver = nullptr;

layout/generic/nsColumnSetFrame.cpp

@@ -657,6 +657,7 @@ nsColumnSetFrame::ColumnBalanceData nsColumnSetFrame::ReflowChildren(
       kidReflowInput.mFlags.mIsTopOfPage = true;
       kidReflowInput.mFlags.mTableIsSplittable = false;
       kidReflowInput.mFlags.mIsColumnBalancing = aConfig.mIsBalancing;
+      kidReflowInput.mBreakType = ReflowInput::BreakType::Column;
 
       // We need to reflow any float placeholders, even if our column block-size
       // hasn't changed.

layout/generic/nsContainerFrame.cpp

@@ -2714,9 +2714,34 @@ bool nsContainerFrame::IsFrameTreeTooDeep(const ReflowInput& aReflowInput,
 bool nsContainerFrame::ShouldAvoidBreakInside(
     const ReflowInput& aReflowInput) const {
   const auto* disp = StyleDisplay();
-  return !aReflowInput.mFlags.mIsTopOfPage &&
-         StyleBreakWithin::Avoid == disp->mBreakInside &&
-         !IsAbsolutelyPositioned(disp) && !GetPrevInFlow();
+  const bool mayAvoidBreak = [&] {
+    switch (disp->mBreakInside) {
+      case StyleBreakWithin::Auto:
+        return false;
+      case StyleBreakWithin::Avoid:
+        return true;
+      case StyleBreakWithin::AvoidPage:
+        return aReflowInput.mBreakType == ReflowInput::BreakType::Page;
+      case StyleBreakWithin::AvoidColumn:
+        return aReflowInput.mBreakType == ReflowInput::BreakType::Column;
+    }
+    MOZ_ASSERT_UNREACHABLE("Unknown break-inside value");
+    return false;
+  }();
+
+  if (!mayAvoidBreak) {
+    return false;
+  }
+  if (aReflowInput.mFlags.mIsTopOfPage) {
+    return false;
+  }
+  if (IsAbsolutelyPositioned(disp)) {
+    return false;
+  }
+  if (GetPrevInFlow()) {
+    return false;
+  }
+  return true;
 }
 
 void nsContainerFrame::ConsiderChildOverflow(OverflowAreas& aOverflowAreas,

layout/generic/nsPageSequenceFrame.cpp

@@ -337,6 +337,8 @@ void nsPageSequenceFrame::Reflow(nsPresContext* aPresContext,
     ReflowInput kidReflowInput(
         aPresContext, aReflowInput, kidFrame,
         LogicalSize(kidFrame->GetWritingMode(), sheetSize));
+    kidReflowInput.mBreakType = ReflowInput::BreakType::Page;
+
     ReflowOutput kidReflowOutput(kidReflowInput);
     nsReflowStatus status;
 

layout/style/test/property_database.js

@@ -7087,15 +7087,18 @@ var gCSSProperties = {
     inherited: false,
     type: CSS_TYPE_LONGHAND,
     initial_values: ["auto"],
-    other_values: ["avoid"],
+    other_values: ["avoid", "avoid-page", "avoid-column"],
     invalid_values: ["left", "right", "always"],
   },
   "page-break-inside": {
     domProp: "pageBreakInside",
     inherited: false,
-    type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
+    type: CSS_TYPE_LEGACY_SHORTHAND,
     alias_for: "break-inside",
     subproperties: ["break-inside"],
+    initial_values: ["auto"],
+    other_values: ["avoid"],
+    invalid_values: ["avoid-page", "avoid-column"],
   },
   "paint-order": {
     domProp: "paintOrder",

servo/components/style/properties/longhands/box.mako.rs

@@ -503,7 +503,6 @@ ${helpers.predefined_type(
     "BreakWithin",
     "computed::BreakWithin::Auto",
     engines="gecko",
-    aliases="page-break-inside",
     spec="https://drafts.csswg.org/css-break/#propdef-break-inside",
     animation_value_type="discrete",
 )}

servo/components/style/properties/shorthands/box.mako.rs

@@ -316,15 +316,15 @@ ${helpers.two_properties_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"
+    spec="https://drafts.csswg.org/css-break-3/#page-break-properties"
 >
     pub fn parse_value<'i>(
-        _: &ParserContext,
+        context: &ParserContext,
         input: &mut Parser<'i, '_>,
     ) -> Result<Longhands, ParseError<'i>> {
         use crate::values::specified::box_::BreakBetween;
         Ok(expanded! {
-            break_before: BreakBetween::parse_legacy(input)?,
+            break_before: BreakBetween::parse_legacy(context, input)?,
         })
     }
 
@@ -340,15 +340,15 @@ ${helpers.two_properties_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"
+    spec="https://drafts.csswg.org/css-break-3/#page-break-properties"
 >
     pub fn parse_value<'i>(
-        _: &ParserContext,
+        context: &ParserContext,
         input: &mut Parser<'i, '_>,
     ) -> Result<Longhands, ParseError<'i>> {
         use crate::values::specified::box_::BreakBetween;
         Ok(expanded! {
-            break_after: BreakBetween::parse_legacy(input)?,
+            break_after: BreakBetween::parse_legacy(context, input)?,
         })
     }
 
@@ -359,6 +359,30 @@ ${helpers.two_properties_shorthand(
     }
 </%helpers:shorthand>
 
+<%helpers:shorthand
+    engines="gecko"
+    name="page-break-inside"
+    flags="SHORTHAND_IN_GETCS IS_LEGACY_SHORTHAND"
+    sub_properties="break-inside"
+    spec="https://drafts.csswg.org/css-break-3/#page-break-properties"
+>
+    pub fn parse_value<'i>(
+        context: &ParserContext,
+        input: &mut Parser<'i, '_>,
+    ) -> Result<Longhands, ParseError<'i>> {
+        use crate::values::specified::box_::BreakWithin;
+        Ok(expanded! {
+            break_inside: BreakWithin::parse_legacy(context, input)?,
+        })
+    }
+
+    impl<'a> ToCss for LonghandsToSerialize<'a> {
+        fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
+            self.break_inside.to_css_legacy(dest)
+        }
+    }
+</%helpers:shorthand>
+
 <%helpers:shorthand name="offset"
                     engines="gecko"
                     sub_properties="offset-path offset-distance offset-rotate offset-anchor"

servo/components/style/values/specified/box.rs

@@ -1882,28 +1882,19 @@ pub enum BreakBetween {
 }
 
 impl BreakBetween {
-    /// Parse a legacy break-between value for `page-break-*`.
+    /// Parse a legacy break-between value for `page-break-{before,after}`.
     ///
     /// 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())));
-            },
-        };
+    pub(crate) fn parse_legacy<'i>(_: &ParserContext, input: &mut Parser<'i, '_>) -> Result<Self, ParseError<'i>> {
+        let break_value = BreakBetween::parse(input)?;
         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())))
+                Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             },
         }
     }
@@ -1911,7 +1902,7 @@ impl BreakBetween {
     /// 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
+    pub(crate) fn to_css_legacy<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
     {
@@ -1948,6 +1939,37 @@ impl BreakBetween {
 pub enum BreakWithin {
     Auto,
     Avoid,
+    AvoidPage,
+    AvoidColumn,
+}
+
+impl BreakWithin {
+    /// Parse a legacy break-between value for `page-break-inside`.
+    ///
+    /// See https://drafts.csswg.org/css-break/#page-break-properties.
+    #[inline]
+    pub(crate) fn parse_legacy<'i>(_: &ParserContext, input: &mut Parser<'i, '_>) -> Result<Self, ParseError<'i>> {
+        let break_value = BreakWithin::parse(input)?;
+        match break_value {
+            BreakWithin::Auto | BreakWithin::Avoid => Ok(break_value),
+            BreakWithin::AvoidPage | BreakWithin::AvoidColumn => {
+                Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
+            },
+        }
+    }
+
+    /// Serialize a legacy break-between value for `page-break-inside`.
+    ///
+    /// See https://drafts.csswg.org/css-break/#page-break-properties.
+    pub(crate) fn to_css_legacy<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        match *self {
+            BreakWithin::Auto | BreakWithin::Avoid => self.to_css(dest),
+            BreakWithin::AvoidPage | BreakWithin::AvoidColumn => Ok(()),
+        }
+    }
 }
 
 /// The value for the `overflow-x` / `overflow-y` properties.

testing/web-platform/meta/css/css-break/parsing/break-inside-computed.html.ini

@@ -1,10 +1,4 @@
 [break-inside-computed.html]
-  [Property break-inside value 'avoid-column']
-    expected: FAIL
-
-  [Property break-inside value 'avoid-page']
-    expected: FAIL
-
   [Property break-inside value 'avoid-region']
     expected: FAIL
 

testing/web-platform/meta/css/css-break/parsing/break-inside-valid.html.ini

@@ -1,10 +1,4 @@
 [break-inside-valid.html]
-  [e.style['break-inside'\] = "avoid-column" should set the property value]
-    expected: FAIL
-
-  [e.style['break-inside'\] = "avoid-page" should set the property value]
-    expected: FAIL
-
   [e.style['break-inside'\] = "avoid-region" should set the property value]
     expected: FAIL
 

testing/web-platform/meta/css/css-multicol/multicol-br-inside-avoidcolumn-001.xht.ini

@@ -1,2 +0,0 @@
-[multicol-br-inside-avoidcolumn-001.xht]
-  expected: FAIL