From 21373b2216fbc56fdb07e35e52c1e5dbab8a39e3 Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Mon, 2 Jan 2023 21:11:30 +0000 Subject: [PATCH] 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 | 4 +-- layout/style/ServoCSSPropList.mako.py | 3 -- layout/style/nsComputedDOMStyle.cpp | 6 ---- layout/style/nsComputedDOMStyle.h | 3 -- layout/style/nsStyleStruct.cpp | 4 ++- layout/style/nsStyleStruct.h | 15 +++++++--- .../components/style/properties/gecko.mako.rs | 29 +++++++++++++++++-- servo/components/style/style_adjuster.rs | 18 ++++++++++-- .../parsing/column-rule-computed.html.ini | 6 ---- 9 files changed, 59 insertions(+), 29 deletions(-) delete mode 100644 testing/web-platform/meta/css/css-multicol/parsing/column-rule-computed.html.ini diff --git a/layout/generic/nsColumnSetFrame.cpp b/layout/generic/nsColumnSetFrame.cpp index dee801909d98..6b4d027466bc 100644 --- a/layout/generic/nsColumnSetFrame.cpp +++ b/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(); diff --git a/layout/style/ServoCSSPropList.mako.py b/layout/style/ServoCSSPropList.mako.py index 89186200286d..be409777a0ac 100644 --- a/layout/style/ServoCSSPropList.mako.py +++ b/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", diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp index 0cc55186774f..3911669c26d5 100644 --- a/layout/style/nsComputedDOMStyle.cpp +++ b/layout/style/nsComputedDOMStyle.cpp @@ -1228,12 +1228,6 @@ already_AddRefed nsComputedDOMStyle::DoGetBottom() { return GetOffsetWidthFor(eSideBottom); } -already_AddRefed nsComputedDOMStyle::DoGetColumnRuleWidth() { - RefPtr val = new nsROCSSPrimitiveValue; - val->SetAppUnits(StyleColumn()->GetComputedColumnRuleWidth()); - return val.forget(); -} - static Position MaybeResolvePositionForTransform(const LengthPercentage& aX, const LengthPercentage& aY, nsIFrame* aInnerFrame) { diff --git a/layout/style/nsComputedDOMStyle.h b/layout/style/nsComputedDOMStyle.h index 16fe0729c49a..213ebe1dc7c9 100644 --- a/layout/style/nsComputedDOMStyle.h +++ b/layout/style/nsComputedDOMStyle.h @@ -270,9 +270,6 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration, already_AddRefed DoGetTransformOrigin(); already_AddRefed DoGetPerspectiveOrigin(); - /* Column properties */ - already_AddRefed DoGetColumnRuleWidth(); - // For working around a MSVC bug. See related comment in // GenerateComputedDOMStyleGenerated.py. already_AddRefed DummyGetter(); diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index f58e3a767927..a9470aeeef79 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/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; diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index c4eae5008c13..65be6e06f197 100644 --- a/layout/style/nsStyleStruct.h +++ b/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 { - return (IsVisibleBorderStyle(mColumnRuleStyle) ? mColumnRuleWidth : 0); - } + nscoord GetColumnRuleWidth() const { return mActualColumnRuleWidth; } 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; }; diff --git a/servo/components/style/properties/gecko.mako.rs b/servo/components/style/properties/gecko.mako.rs index 05f2d3223bd8..63316c09b07f 100644 --- a/servo/components/style/properties/gecko.mako.rs +++ b/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 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", diff --git a/servo/components/style/style_adjuster.rs b/servo/components/style/style_adjuster.rs index 596bf419f672..dd135c0abd58 100644 --- a/servo/components/style/style_adjuster.rs +++ b/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")] { diff --git a/testing/web-platform/meta/css/css-multicol/parsing/column-rule-computed.html.ini b/testing/web-platform/meta/css/css-multicol/parsing/column-rule-computed.html.ini deleted file mode 100644 index 8f67d8f2475b..000000000000 --- a/testing/web-platform/meta/css/css-multicol/parsing/column-rule-computed.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[column-rule-computed.html] - [Property column-rule value '10px'] - expected: FAIL - - [Property column-rule value 'medium hidden currentcolor'] - expected: FAIL