Skip to content

Commit 25098da

Browse files
emilioemilio
committed
Bug 1532122 - Make word-spacing, letter-spacing, and line-height use Rust lengths. r=boris
This also adopts the resolution from [1] while at it, making letter-spacing compute to a length, serializing 0 to normal rather than keeping normal in the computed value, which matches every other engine. This removes the SMIL tests for percentages from letter-spacing since letter-spacing does in fact not support percentages, so they were passing just by chance. [1]: w3c/csswg-drafts#1484 Differential Revision: https://phabricator.services.mozilla.com/D21850 --HG-- extra : moz-landing-system : lando
1 parent 2a809aa commit 25098da

22 files changed

+170
-317
lines changed

‎dom/smil/test/db_smilCSSFromTo.js

+13-7
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,16 @@ var _fromToTestLists = {
149149
],
150150
};
151151

152+
function _tweakForLetterSpacing(testcases) {
153+
return testcases.map(function(t) {
154+
let valMap = Object.assign({}, t.computedValMap);
155+
for (let prop of Object.keys(valMap))
156+
if (valMap[prop] == "0px")
157+
valMap[prop] = "normal";
158+
return new AnimTestcaseFromTo(t.from, t.to, valMap)
159+
});
160+
}
161+
152162
// List of attribute/testcase-list bundles to be tested
153163
var gFromToBundles = [
154164
new TestcaseBundle(gPropList.clip, [
@@ -341,13 +351,9 @@ var gFromToBundles = [
341351
toComp: "optimizespeed" }),
342352
]),
343353
new TestcaseBundle(gPropList.letter_spacing,
344-
[].concat(_fromToTestLists.lengthNoUnits,
345-
_fromToTestLists.lengthPx,
346-
_fromToTestLists.lengthPxPctSVG)),
347-
new TestcaseBundle(gPropList.letter_spacing,
348-
_fromToTestLists.lengthPctSVG,
349-
"pct->pct animations don't currently work for " +
350-
"*-spacing properties"),
354+
_tweakForLetterSpacing(
355+
[].concat(_fromToTestLists.lengthNoUnits,
356+
_fromToTestLists.lengthPx))),
351357
new TestcaseBundle(gPropList.lighting_color,
352358
[].concat(_fromToTestLists.color,
353359
_fromToTestLists.colorFromInheritWhite)),

‎layout/forms/nsTextControlFrame.cpp

+3-6
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,9 @@ LogicalSize nsTextControlFrame::CalcIntrinsicSize(
201201

202202
// Increment width with cols * letter-spacing.
203203
{
204-
const nsStyleCoord& lsCoord = StyleText()->mLetterSpacing;
205-
if (eStyleUnit_Coord == lsCoord.GetUnit()) {
206-
nscoord letterSpacing = lsCoord.GetCoordValue();
207-
if (letterSpacing != 0) {
208-
intrinsicSize.ISize(aWM) += cols * letterSpacing;
209-
}
204+
const StyleLength& letterSpacing = StyleText()->mLetterSpacing;
205+
if (!letterSpacing.IsZero()) {
206+
intrinsicSize.ISize(aWM) += cols * letterSpacing.ToAppUnits();
210207
}
211208
}
212209

‎layout/generic/ReflowInput.cpp

+10-21
Original file line numberDiff line numberDiff line change
@@ -734,11 +734,7 @@ void ReflowInput::InitResizeFlags(nsPresContext* aPresContext,
734734
mStylePosition->OffsetHasPercent(wm.PhysicalSide(eLogicalSideBStart)) ||
735735
!mStylePosition->mOffset.GetBEnd(wm).IsAuto() || mFrame->IsXULBoxFrame();
736736

737-
if (mStyleText->mLineHeight.GetUnit() == eStyleUnit_Enumerated) {
738-
NS_ASSERTION(mStyleText->mLineHeight.GetIntValue() ==
739-
NS_STYLE_LINE_HEIGHT_BLOCK_HEIGHT,
740-
"bad line-height value");
741-
737+
if (mStyleText->mLineHeight.IsMozBlockHeight()) {
742738
// line-height depends on block bsize
743739
mFrame->AddStateBits(NS_FRAME_CONTAINS_RELATIVE_BSIZE);
744740
// but only on containing blocks if this frame is not a suitable block
@@ -2849,33 +2845,26 @@ static inline nscoord ComputeLineHeight(ComputedStyle* aComputedStyle,
28492845
nsPresContext* aPresContext,
28502846
nscoord aBlockBSize,
28512847
float aFontSizeInflation) {
2852-
const nsStyleCoord& lhCoord = aComputedStyle->StyleText()->mLineHeight;
2853-
2854-
if (lhCoord.GetUnit() == eStyleUnit_Coord) {
2855-
nscoord result = lhCoord.GetCoordValue();
2848+
const StyleLineHeight& lineHeight = aComputedStyle->StyleText()->mLineHeight;
2849+
if (lineHeight.IsLength()) {
2850+
nscoord result = lineHeight.length._0.ToAppUnits();
28562851
if (aFontSizeInflation != 1.0f) {
28572852
result = NSToCoordRound(result * aFontSizeInflation);
28582853
}
28592854
return result;
28602855
}
28612856

2862-
if (lhCoord.GetUnit() == eStyleUnit_Factor)
2857+
if (lineHeight.IsNumber()) {
28632858
// For factor units the computed value of the line-height property
28642859
// is found by multiplying the factor by the font's computed size
28652860
// (adjusted for min-size prefs and text zoom).
2866-
return NSToCoordRound(lhCoord.GetFactorValue() * aFontSizeInflation *
2861+
return NSToCoordRound(lineHeight.number._0 * aFontSizeInflation *
28672862
aComputedStyle->StyleFont()->mFont.size);
2863+
}
28682864

2869-
NS_ASSERTION(lhCoord.GetUnit() == eStyleUnit_Normal ||
2870-
lhCoord.GetUnit() == eStyleUnit_Enumerated,
2871-
"bad line-height unit");
2872-
2873-
if (lhCoord.GetUnit() == eStyleUnit_Enumerated) {
2874-
NS_ASSERTION(lhCoord.GetIntValue() == NS_STYLE_LINE_HEIGHT_BLOCK_HEIGHT,
2875-
"bad line-height value");
2876-
if (aBlockBSize != NS_AUTOHEIGHT) {
2877-
return aBlockBSize;
2878-
}
2865+
MOZ_ASSERT(lineHeight.IsNormal() || lineHeight.IsMozBlockHeight());
2866+
if (lineHeight.IsMozBlockHeight() && aBlockBSize != NS_AUTOHEIGHT) {
2867+
return aBlockBSize;
28792868
}
28802869

28812870
RefPtr<nsFontMetrics> fm = nsLayoutUtils::GetFontMetricsForComputedStyle(

‎layout/generic/nsLineLayout.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1877,7 +1877,7 @@ void nsLineLayout::VerticalAlignFrames(PerSpanData* psd) {
18771877
// the frame block size if the user has left line-height == normal
18781878
const nsStyleText* styleText = spanFrame->StyleText();
18791879
if (spanFramePFD->mIsLetterFrame && !spanFrame->GetPrevInFlow() &&
1880-
styleText->mLineHeight.GetUnit() == eStyleUnit_Normal) {
1880+
styleText->mLineHeight.IsNormal()) {
18811881
logicalBSize = spanFramePFD->mBounds.BSize(lineWM);
18821882
}
18831883

@@ -2191,7 +2191,7 @@ void nsLineLayout::VerticalAlignFrames(PerSpanData* psd) {
21912191
// line-height=normal.
21922192
canUpdate =
21932193
pfd->mIsNonWhitespaceTextFrame &&
2194-
frame->StyleText()->mLineHeight.GetUnit() == eStyleUnit_Normal;
2194+
frame->StyleText()->mLineHeight.IsNormal();
21952195
} else {
21962196
canUpdate = !pfd->mIsPlaceholder;
21972197
}

‎layout/generic/nsTextFrame.cpp

+8-22
Original file line numberDiff line numberDiff line change
@@ -1729,7 +1729,7 @@ static gfxFont::Metrics GetFirstFontMetrics(gfxFontGroup* aFontGroup,
17291729
: gfxFont::eHorizontal);
17301730
}
17311731

1732-
static gfxFloat GetSpaceWidthAppUnits(const gfxTextRun* aTextRun) {
1732+
static nscoord GetSpaceWidthAppUnits(const gfxTextRun* aTextRun) {
17331733
// Round the space width when converting to appunits the same way textruns
17341734
// do.
17351735
gfxFloat spaceWidthAppUnits =
@@ -1757,12 +1757,7 @@ static nscoord LetterSpacing(nsIFrame* aFrame,
17571757
if (!aStyleText) {
17581758
aStyleText = aFrame->StyleText();
17591759
}
1760-
1761-
const nsStyleCoord& coord = aStyleText->mLetterSpacing;
1762-
if (eStyleUnit_Coord == coord.GetUnit()) {
1763-
return coord.GetCoordValue();
1764-
}
1765-
return 0;
1760+
return aStyleText->mLetterSpacing.ToAppUnits();
17661761
}
17671762

17681763
// This function converts non-coord values (e.g. percentages) to nscoord.
@@ -1775,12 +1770,9 @@ static nscoord WordSpacing(nsIFrame* aFrame, const gfxTextRun* aTextRun,
17751770
aStyleText = aFrame->StyleText();
17761771
}
17771772

1778-
const nsStyleCoord& coord = aStyleText->mWordSpacing;
1779-
if (coord.IsCoordPercentCalcUnit()) {
1780-
nscoord pctBasis = coord.HasPercent() ? GetSpaceWidthAppUnits(aTextRun) : 0;
1781-
return coord.ComputeCoordPercentCalc(pctBasis);
1782-
}
1783-
return 0;
1773+
return aStyleText->mWordSpacing.Resolve([&] {
1774+
return GetSpaceWidthAppUnits(aTextRun);
1775+
});
17841776
}
17851777

17861778
// Returns gfxTextRunFactory::TEXT_ENABLE_SPACING if non-standard
@@ -1792,20 +1784,14 @@ static gfx::ShapedTextFlags GetSpacingFlags(
17921784
}
17931785

17941786
const nsStyleText* styleText = aFrame->StyleText();
1795-
const nsStyleCoord& ls = styleText->mLetterSpacing;
1796-
const nsStyleCoord& ws = styleText->mWordSpacing;
1787+
const auto& ls = styleText->mLetterSpacing;
1788+
const auto& ws = styleText->mWordSpacing;
17971789

17981790
// It's possible to have a calc() value that computes to zero but for which
17991791
// IsDefinitelyZero() is false, in which case we'll return
18001792
// TEXT_ENABLE_SPACING unnecessarily. That's ok because such cases are likely
18011793
// to be rare, and avoiding TEXT_ENABLE_SPACING is just an optimization.
1802-
bool nonStandardSpacing =
1803-
(eStyleUnit_Coord == ls.GetUnit() && ls.GetCoordValue() != 0) ||
1804-
(eStyleUnit_Coord == ws.GetUnit() && ws.GetCoordValue() != 0) ||
1805-
(eStyleUnit_Percent == ws.GetUnit() && ws.GetPercentValue() != 0) ||
1806-
(eStyleUnit_Calc == ws.GetUnit() &&
1807-
!ws.GetCalcValue()->IsDefinitelyZero());
1808-
1794+
bool nonStandardSpacing = !ls.IsZero() || !ws.IsDefinitelyZero();
18091795
return nonStandardSpacing ? gfx::ShapedTextFlags::TEXT_ENABLE_SPACING
18101796
: gfx::ShapedTextFlags();
18111797
}

‎layout/style/ServoBindings.toml

+2
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,8 @@ cbindgen-types = [
446446
{ gecko = "StyleGenericZIndex", servo = "values::generics::position::ZIndex" },
447447
{ gecko = "StyleTransformOrigin", servo = "values::computed::TransformOrigin" },
448448
{ gecko = "StyleGenericBorderRadius", servo = "values::generics::border::BorderRadius" },
449+
{ gecko = "StyleLetterSpacing", servo = "values::computed::text::LetterSpacing" },
450+
{ gecko = "StyleGenericLineHeight", servo = "values::generics::text::LineHeight" },
449451
]
450452

451453
mapped-generic-types = [

‎layout/style/ServoCSSPropList.mako.py

-2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ def method(prop):
101101
"grid-row-start",
102102
"grid-template-areas",
103103
"initial-letter",
104-
"letter-spacing",
105104
"marker-end",
106105
"marker-mid",
107106
"marker-start",
@@ -140,7 +139,6 @@ def method(prop):
140139
"vertical-align",
141140
"-webkit-text-stroke-width",
142141
"will-change",
143-
"word-spacing",
144142
]
145143

146144
def serialized_by_servo(prop):

‎layout/style/nsComputedDOMStyle.cpp

+18-19
Original file line numberDiff line numberDiff line change
@@ -2012,14 +2012,25 @@ already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetInitialLetter() {
20122012
already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetLineHeight() {
20132013
RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
20142014

2015-
nscoord lineHeight;
2016-
if (GetLineHeightCoord(lineHeight)) {
2017-
val->SetAppUnits(lineHeight);
2018-
} else {
2019-
SetValueToCoord(val, StyleText()->mLineHeight, true, nullptr,
2020-
nsCSSProps::kLineHeightKTable);
2015+
{
2016+
nscoord lineHeight;
2017+
if (GetLineHeightCoord(lineHeight)) {
2018+
val->SetAppUnits(lineHeight);
2019+
return val.forget();
2020+
}
20212021
}
20222022

2023+
auto& lh = StyleText()->mLineHeight;
2024+
if (lh.IsLength()) {
2025+
val->SetAppUnits(lh.length._0.ToAppUnits());
2026+
} else if (lh.IsNumber()) {
2027+
val->SetNumber(lh.number._0);
2028+
} else if (lh.IsMozBlockHeight()) {
2029+
val->SetIdent(eCSSKeyword__moz_block_height);
2030+
} else {
2031+
MOZ_ASSERT(lh.IsNormal());
2032+
val->SetIdent(eCSSKeyword_normal);
2033+
}
20232034
return val.forget();
20242035
}
20252036

@@ -2188,18 +2199,6 @@ already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetTextShadow() {
21882199
return GetCSSShadowArray(StyleText()->mTextShadow, false);
21892200
}
21902201

2191-
already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetLetterSpacing() {
2192-
RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
2193-
SetValueToCoord(val, StyleText()->mLetterSpacing, false);
2194-
return val.forget();
2195-
}
2196-
2197-
already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetWordSpacing() {
2198-
RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
2199-
SetValueToCoord(val, StyleText()->mWordSpacing, false);
2200-
return val.forget();
2201-
}
2202-
22032202
already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetWebkitTextStrokeWidth() {
22042203
RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
22052204
val->SetAppUnits(StyleText()->mWebkitTextStrokeWidth);
@@ -2674,7 +2673,7 @@ bool nsComputedDOMStyle::GetLineHeightCoord(nscoord& aCoord) {
26742673
AssertFlushedPendingReflows();
26752674

26762675
nscoord blockHeight = NS_AUTOHEIGHT;
2677-
if (StyleText()->mLineHeight.GetUnit() == eStyleUnit_Enumerated) {
2676+
if (StyleText()->mLineHeight.IsMozBlockHeight()) {
26782677
if (!mInnerFrame) return false;
26792678

26802679
if (nsLayoutUtils::IsNonWrapperBlock(mInnerFrame)) {

‎layout/style/nsComputedDOMStyle.h

-6
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,6 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
294294
already_AddRefed<CSSValue> DoGetBorderTopLeftRadius();
295295
already_AddRefed<CSSValue> DoGetBorderTopRightRadius();
296296

297-
298297
/* Border Image */
299298
already_AddRefed<CSSValue> DoGetBorderImageWidth();
300299

@@ -314,7 +313,6 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
314313
already_AddRefed<CSSValue> DoGetOutlineRadiusTopLeft();
315314
already_AddRefed<CSSValue> DoGetOutlineRadiusTopRight();
316315

317-
318316
/* Text Properties */
319317
already_AddRefed<CSSValue> DoGetInitialLetter();
320318
already_AddRefed<CSSValue> DoGetLineHeight();
@@ -326,8 +324,6 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
326324
already_AddRefed<CSSValue> DoGetTextEmphasisStyle();
327325
already_AddRefed<CSSValue> DoGetTextOverflow();
328326
already_AddRefed<CSSValue> DoGetTextShadow();
329-
already_AddRefed<CSSValue> DoGetLetterSpacing();
330-
already_AddRefed<CSSValue> DoGetWordSpacing();
331327
already_AddRefed<CSSValue> DoGetWebkitTextStrokeWidth();
332328

333329
/* Display properties */
@@ -379,8 +375,6 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
379375
already_AddRefed<CSSValue> DoGetMarkerMid();
380376
already_AddRefed<CSSValue> DoGetMarkerStart();
381377

382-
383-
384378
already_AddRefed<CSSValue> DoGetFilter();
385379
already_AddRefed<CSSValue> DoGetPaintOrder();
386380

‎layout/style/nsStyleCoord.h

+9-1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ using NonNegativeLengthPercentageOrAuto =
4646
StyleNonNegativeLengthPercentageOrAuto;
4747
using BorderRadius = StyleBorderRadius;
4848

49+
bool StyleCSSPixelLength::IsZero() const {
50+
return _0 == 0.0f;
51+
}
52+
4953
nscoord StyleCSSPixelLength::ToAppUnits() const {
5054
// We want to resolve the length part of the calc() expression rounding 0.5
5155
// away from zero, instead of the default behavior of NSToCoordRoundWithClamp
@@ -95,7 +99,7 @@ nscoord LengthPercentage::ToLength() const {
9599
}
96100

97101
bool LengthPercentage::ConvertsToPercentage() const {
98-
return has_percentage && length._0 == 0.0f;
102+
return has_percentage && length.IsZero();
99103
}
100104

101105
float LengthPercentage::ToPercentage() const {
@@ -107,6 +111,10 @@ bool LengthPercentage::HasLengthAndPercentage() const {
107111
return !ConvertsToLength() && !ConvertsToPercentage();
108112
}
109113

114+
bool LengthPercentage::IsDefinitelyZero() const {
115+
return length.IsZero() && Percentage() == 0.0f;
116+
}
117+
110118
CSSCoord LengthPercentage::ResolveToCSSPixels(CSSCoord aPercentageBasis) const {
111119
return LengthInCSSPixels() + Percentage() * aPercentageBasis;
112120
}

‎layout/style/nsStyleStruct.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -3711,9 +3711,9 @@ nsStyleText::nsStyleText(const Document& aDocument)
37113711
mWebkitTextStrokeColor(StyleComplexColor::CurrentColor()),
37123712
mMozTabSize(
37133713
StyleNonNegativeLengthOrNumber::Number(NS_STYLE_TABSIZE_INITIAL)),
3714-
mWordSpacing(0, nsStyleCoord::CoordConstructor),
3715-
mLetterSpacing(eStyleUnit_Normal),
3716-
mLineHeight(eStyleUnit_Normal),
3714+
mWordSpacing(LengthPercentage::Zero()),
3715+
mLetterSpacing({0.}),
3716+
mLineHeight(StyleLineHeight::Normal()),
37173717
mTextIndent(LengthPercentage::Zero()),
37183718
mWebkitTextStrokeWidth(0),
37193719
mTextShadow(nullptr) {

‎layout/style/nsStyleStruct.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -1476,9 +1476,9 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleText {
14761476
mozilla::StyleComplexColor mWebkitTextStrokeColor;
14771477

14781478
mozilla::StyleNonNegativeLengthOrNumber mMozTabSize;
1479-
nsStyleCoord mWordSpacing; // coord, percent, calc
1480-
nsStyleCoord mLetterSpacing; // coord, normal
1481-
nsStyleCoord mLineHeight; // coord, factor, normal
1479+
mozilla::LengthPercentage mWordSpacing;
1480+
mozilla::StyleLetterSpacing mLetterSpacing;
1481+
mozilla::StyleLineHeight mLineHeight;
14821482
mozilla::LengthPercentage mTextIndent;
14831483
nscoord mWebkitTextStrokeWidth; // coord
14841484

‎layout/style/test/property_database.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -3962,11 +3962,9 @@ var gCSSProperties = {
39623962
applies_to_first_letter: true,
39633963
applies_to_first_line: true,
39643964
applies_to_placeholder: true,
3965-
initial_values: [ "normal" ],
3966-
other_values: [ "0", "0px", "1em", "2px", "-3px",
3967-
"calc(0px)", "calc(1em)", "calc(1em + 3px)",
3968-
"calc(15px / 2)", "calc(15px/2)", "calc(-3px)"
3969-
],
3965+
initial_values: [ "normal", "0", "0px", "calc(0px)" ],
3966+
other_values: [ "1em", "2px", "-3px", "calc(1em)", "calc(1em + 3px)",
3967+
"calc(15px / 2)", "calc(15px/2)", "calc(-3px)" ],
39703968
invalid_values: [],
39713969
quirks_values: { "5": "5px" },
39723970
},

‎layout/style/test/test_transitions_per_property.html

+2-3
Original file line numberDiff line numberDiff line change
@@ -1284,11 +1284,10 @@
12841284
div.style.setProperty("transition-timing-function", FUNC_NEGATIVE, "");
12851285
div.style.setProperty("transition-property", "none", "");
12861286
div.style.setProperty(prop, "0px", "");
1287-
is(cs.getPropertyValue(prop), "0px",
1288-
"length-valued property " + prop + ": flush before clamping test");
1287+
let zero_val = cs.getPropertyValue(prop); // Flushes
12891288
div.style.setProperty("transition-property", prop, "");
12901289
div.style.setProperty(prop, "100px", "");
1291-
(is_clamped ? is : isnot)(cs.getPropertyValue(prop), "0px",
1290+
(is_clamped ? is : isnot)(cs.getPropertyValue(prop), zero_val,
12921291
"length-valued property " + prop + ": clamping of negatives");
12931292
div.style.setProperty("transition-timing-function", "linear", "");
12941293
}

0 commit comments

Comments
 (0)