Bug 1800952 - Compute column-rule-width to 0 when column-rule-style is none. r=emilio

Depends on D164554

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

layout/generic/nsColumnSetFrame.cpp

@@ -126,7 +126,7 @@ void nsColumnSetFrame::ForEachColumnRule(
   if (!nextSibling) return;  // 1 column only - this means no gap to draw on
 
   const nsStyleColumn* colStyle = StyleColumn();
-  nscoord ruleWidth = colStyle->GetComputedColumnRuleWidth();
+  nscoord ruleWidth = colStyle->GetColumnRuleWidth();
   if (!ruleWidth) return;
 
   WritingMode wm = GetWritingMode();
@@ -184,7 +184,7 @@ void nsColumnSetFrame::CreateBorderRenderers(
   else
     ruleStyle = colStyle->mColumnRuleStyle;
 
-  nscoord ruleWidth = colStyle->GetComputedColumnRuleWidth();
+  nscoord ruleWidth = colStyle->GetColumnRuleWidth();
   if (!ruleWidth) return;
 
   aBorderRenderers.Clear();

layout/style/ServoCSSPropList.mako.py

@@ -58,9 +58,6 @@ LONGHANDS_NOT_SERIALIZED_WITH_SERVO = [
     # Servo serializes one value when both are the same, a few tests expect two.
     "border-spacing",
 
-    # Resolved value should be zero when the column-rule-style is none.
-    "column-rule-width",
-
     # These resolve auto to zero in a few cases, but not all.
     "max-height",
     "max-width",

layout/style/nsComputedDOMStyle.cpp

@@ -1228,12 +1228,6 @@ already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetBottom() {
   return GetOffsetWidthFor(eSideBottom);
 }
 
-already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetColumnRuleWidth() {
-  RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
-  val->SetAppUnits(StyleColumn()->GetComputedColumnRuleWidth());
-  return val.forget();
-}
-
 static Position MaybeResolvePositionForTransform(const LengthPercentage& aX,
                                                  const LengthPercentage& aY,
                                                  nsIFrame* aInnerFrame) {

layout/style/nsComputedDOMStyle.h

@@ -270,9 +270,6 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
   already_AddRefed<CSSValue> DoGetTransformOrigin();
   already_AddRefed<CSSValue> DoGetPerspectiveOrigin();
 
-  /* Column properties */
-  already_AddRefed<CSSValue> DoGetColumnRuleWidth();
-
   // For working around a MSVC bug. See related comment in
   // GenerateComputedDOMStyleGenerated.py.
   already_AddRefed<CSSValue> DummyGetter();

layout/style/nsStyleStruct.cpp

@@ -724,6 +724,7 @@ nsStyleColumn::nsStyleColumn(const Document& aDocument)
       mColumnRuleColor(StyleColor::CurrentColor()),
       mColumnRuleStyle(StyleBorderStyle::None),
       mColumnRuleWidth(kMediumBorderWidth),
+      mActualColumnRuleWidth(0),
       mTwipsPerPixel(TwipsPerPixel(aDocument)) {
   MOZ_COUNT_CTOR(nsStyleColumn);
 }
@@ -738,6 +739,7 @@ nsStyleColumn::nsStyleColumn(const nsStyleColumn& aSource)
       mColumnFill(aSource.mColumnFill),
       mColumnSpan(aSource.mColumnSpan),
       mColumnRuleWidth(aSource.mColumnRuleWidth),
+      mActualColumnRuleWidth(aSource.mActualColumnRuleWidth),
       mTwipsPerPixel(aSource.mTwipsPerPixel) {
   MOZ_COUNT_CTOR(nsStyleColumn);
 }
@@ -759,7 +761,7 @@ nsChangeHint nsStyleColumn::CalcDifference(
     return NS_STYLE_HINT_REFLOW;
   }
 
-  if (GetComputedColumnRuleWidth() != aNewData.GetComputedColumnRuleWidth() ||
+  if (mActualColumnRuleWidth != aNewData.mActualColumnRuleWidth ||
       mColumnRuleStyle != aNewData.mColumnRuleStyle ||
       mColumnRuleColor != aNewData.mColumnRuleColor) {
     return NS_STYLE_HINT_VISUAL;

layout/style/nsStyleStruct.h

@@ -1987,9 +1987,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleColumn {
   mozilla::StyleColumnFill mColumnFill = mozilla::StyleColumnFill::Balance;
   mozilla::StyleColumnSpan mColumnSpan = mozilla::StyleColumnSpan::None;
 
-  nscoord GetComputedColumnRuleWidth() const {
+  nscoord GetColumnRuleWidth() const { return mActualColumnRuleWidth; }
-    return (IsVisibleBorderStyle(mColumnRuleStyle) ? mColumnRuleWidth : 0);
-  }
 
   bool IsColumnContainerStyle() const {
     return mColumnCount != kColumnCountAuto || !mColumnWidth.IsAuto();
@@ -2000,7 +1998,16 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleColumn {
   }
 
  protected:
-  nscoord mColumnRuleWidth;  // coord
+  // This is the specified value of column-rule-width, but with length values
+  // computed to absolute.  mActualColumnRuleWidth stores the column-rule-width
+  // value used by layout.  (We must store mColumnRuleWidth for the same
+  // style struct resolution reasons that we do nsStyleBorder::mBorder;
+  // see that field's comment.)
+  nscoord mColumnRuleWidth;
+  // The actual value of column-rule-width is the computed value (an absolute
+  // length, forced to zero when column-rule-style is none) rounded to device
+  // pixels.  This is the value used by layout.
+  nscoord mActualColumnRuleWidth;
   nscoord mTwipsPerPixel;
 };
 

servo/components/style/properties/gecko.mako.rs

@@ -1725,9 +1725,33 @@ mask-mode mask-repeat mask-clip mask-origin mask-composite mask-position-x mask-
         }
     }
 
-    <% impl_non_negative_length("column_rule_width", "mColumnRuleWidth",
+    pub fn set_column_rule_style(&mut self, v: longhands::column_rule_style::computed_value::T) {
+        self.gecko.mColumnRuleStyle = v;
+        // NB: This is needed to correctly handling the initial value of
+        // column-rule-width when colun-rule-style changes, see the
+        // update_border_${side.ident} comment for more details.
+        self.gecko.mActualColumnRuleWidth = self.gecko.mColumnRuleWidth;
+    }
+
+    pub fn copy_column_rule_style_from(&mut self, other: &Self) {
+        self.set_column_rule_style(other.gecko.mColumnRuleStyle);
+    }
+
+    pub fn reset_column_rule_style(&mut self, other: &Self) {
+        self.copy_column_rule_style_from(other)
+    }
+
+    pub fn clone_column_rule_style(&self) -> longhands::column_rule_style::computed_value::T {
+        self.gecko.mColumnRuleStyle.clone()
+    }
+
+    <% impl_non_negative_length("column_rule_width", "mActualColumnRuleWidth",
+                                inherit_from="mColumnRuleWidth",
                                 round_to_pixels=True) %>
-    ${impl_simple('column_rule_style', 'mColumnRuleStyle')}
+
+    pub fn column_rule_has_nonzero_width(&self) -> bool {
+        self.gecko.mActualColumnRuleWidth != 0
+    }
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="Counters">
@@ -2008,6 +2032,7 @@ pub fn assert_initial_values_match(data: &PerDocumentStyleData) {
                 "border-bottom-width",
                 "border-left-width",
                 "border-right-width",
+                "column-rule-width",
                 "font-family",
                 "font-size",
                 "outline-width",

servo/components/style/style_adjuster.rs

@@ -434,9 +434,22 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         properties::adjust_border_width(self.style);
     }
 
+    /// column-rule-style: none causes a computed column-rule-width of zero
+    /// at computed value time.
+    fn adjust_for_column_rule_width(&mut self) {
+        let column_style = self.style.get_column();
+        if !column_style.clone_column_rule_style().none_or_hidden() {
+            return;
+        }
+        if !column_style.column_rule_has_nonzero_width() {
+            return;
+        }
+        self.style.mutate_column().set_column_rule_width(crate::Zero::zero());
+    }
+
     /// outline-style: none causes a computed outline-width of zero at computed
     /// value time.
-    fn adjust_for_outline(&mut self) {
+    fn adjust_for_outline_width(&mut self) {
         let outline = self.style.get_outline();
         if !outline.clone_outline_style().none_or_hidden() {
             return;
@@ -950,7 +963,8 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             self.adjust_for_alignment(layout_parent_style);
         }
         self.adjust_for_border_width();
-        self.adjust_for_outline();
+        self.adjust_for_column_rule_width();
+        self.adjust_for_outline_width();
         self.adjust_for_writing_mode(layout_parent_style);
         #[cfg(feature = "gecko")]
         {

testing/web-platform/meta/css/css-multicol/parsing/column-rule-computed.html.ini

@@ -1,6 +0,0 @@
-[column-rule-computed.html]
-  [Property column-rule value '10px']
-    expected: FAIL
-
-  [Property column-rule value 'medium hidden currentcolor']
-    expected: FAIL