diff --git a/devtools/shared/css/generated/properties-db.js b/devtools/shared/css/generated/properties-db.js index 55362e126d70..3c0fa80cae97 100644 --- a/devtools/shared/css/generated/properties-db.js +++ b/devtools/shared/css/generated/properties-db.js @@ -10157,7 +10157,6 @@ exports.CSS_PROPERTIES = { "supports": [], "values": [ "auto", - "from-font", "inherit", "initial", "revert", @@ -10172,6 +10171,7 @@ exports.CSS_PROPERTIES = { "supports": [], "values": [ "auto", + "from-font", "inherit", "initial", "left", @@ -10804,22 +10804,22 @@ exports.PREFERENCES = [ "scrollbar-color", "layout.css.scrollbar-color.enabled" ], - [ - "translate", - "layout.css.individual-transform.enabled" - ], [ "text-decoration-thickness", "layout.css.text-decoration-thickness.enabled" ], [ - "text-underline-offset", - "layout.css.text-underline-offset.enabled" + "translate", + "layout.css.individual-transform.enabled" ], [ "offset-distance", "layout.css.motion-path.enabled" ], + [ + "text-underline-offset", + "layout.css.text-underline-offset.enabled" + ], [ "overflow-clip-box", "layout.css.overflow-clip-box.enabled" diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index 8c08d820431e..cce634d4757e 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -4953,10 +4953,10 @@ static nscoord LazyGetLineBaselineOffset(nsIFrame* aChildFrame, } } -static bool IsUnderlineRight(nsIFrame* aFrame) { +static bool IsUnderlineRight(const ComputedStyle& aStyle) { // Check for 'left' or 'right' explicitly specified in the property; // if neither is there, we use auto positioning based on lang. - const auto position = aFrame->StyleText()->mTextUnderlinePosition; + const auto position = aStyle.StyleText()->mTextUnderlinePosition; if (position.IsLeft()) { return false; } @@ -4964,7 +4964,7 @@ static bool IsUnderlineRight(nsIFrame* aFrame) { return true; } // If neither 'left' nor 'right' was specified, check the language. - nsAtom* langAtom = aFrame->StyleFont()->mLanguage; + nsAtom* langAtom = aStyle.StyleFont()->mLanguage; if (!langAtom) { return false; } @@ -5082,7 +5082,7 @@ void nsTextFrame::GetTextDecorations( } bool swapUnderlineAndOverline = - vertical && !wm.IsSideways() && IsUnderlineRight(f); + vertical && !wm.IsSideways() && IsUnderlineRight(*context); const auto kUnderline = swapUnderlineAndOverline ? StyleTextDecorationLine::OVERLINE : StyleTextDecorationLine::UNDERLINE; @@ -5304,28 +5304,26 @@ static void SetWidthIfLength( // this function // On entry, aParams.offset is already initialized with the underlineOffset // from the font; this function may adjust it as appropriate. -static void SetOffsetIfLength(const StyleTextDecorationLength& aOffset, +static void SetOffsetIfLength(const StyleTextUnderlinePosition& aPosition, + const LengthPercentageOrAuto& aOffset, nsCSSRendering::DecorationRectParams& aParams, const gfxFont::Metrics& aFontMetrics, const gfxFloat aAppUnitsPerDevPixel, bool aIsSideways, bool aRightUnderline) { - // `auto` is treated like `from-font`, except that (for horizontal/sideways - // text) we clamp the offset to a minimum of 1/16 em (equivalent to 1px at - // font-size 16px) to mitigate skip-ink issues with fonts that leave the - // underlineOffset field as zero. + // If offset is `auto`, we clamp the offset (in horizontal typographic mode) + // to a minimum of 1/16 em (equivalent to 1px at font-size 16px) to mitigate + // skip-ink issues with fonts that leave the underlineOffset field as zero. if (aOffset.IsAuto()) { if (!aParams.vertical || aIsSideways) { aParams.offset = std::min(aParams.offset, gfx::Float(-aFontMetrics.emHeight / 16.0)); + if (aPosition.IsUnder()) { + aParams.offset -= aFontMetrics.maxDescent; + } } return; } - // If the value is `from-font`, just leave the font's value untouched. - if (aOffset.IsFromFont()) { - return; - } - auto em = [&] { return nscoord(NS_round(aFontMetrics.emHeight * aAppUnitsPerDevPixel)); }; @@ -5345,8 +5343,21 @@ static void SetOffsetIfLength(const StyleTextDecorationLength& aOffset, aParams.offset += aOffset.AsLengthPercentage().Resolve(em) / aAppUnitsPerDevPixel; } else { - aParams.offset = - -aOffset.AsLengthPercentage().Resolve(em) / aAppUnitsPerDevPixel; + const gfxFloat resolvedOffset = + aOffset.AsLengthPercentage().Resolve(em) / aAppUnitsPerDevPixel; + if (aPosition.IsAuto()) { + // 'zero position' for text-underline-offset:auto is the alphabetic + // baseline, so we forget the input offset value (from the font). + aParams.offset = -resolvedOffset; + } else if (aPosition.IsUnder()) { + // 'zero position' for text-underline-offset is the font's descender + // metric. + aParams.offset = -aFontMetrics.maxDescent - resolvedOffset; + } else { + // 'zero position' is the from-font value, so we apply offset to that. + MOZ_ASSERT(aPosition.IsFromFont()); + aParams.offset -= resolvedOffset; + } } } @@ -5382,10 +5393,10 @@ void nsTextFrame::UnionAdditionalOverflow(nsPresContext* aPresContext, // underline position from the font. const auto* styleText = aBlock->StyleText(); if (styleText->mTextUnderlinePosition.IsUnder()) { - underlineOffset = -fontMetrics->EmDescent(); + underlineOffset -= fontMetrics->MaxDescent(); } - const StyleTextDecorationLength& textUnderlineOffset = + const LengthPercentageOrAuto& textUnderlineOffset = styleText->mTextUnderlineOffset; const StyleTextDecorationLength& textDecorationThickness = @@ -5397,6 +5408,7 @@ void nsTextFrame::UnionAdditionalOverflow(nsPresContext* aPresContext, underlineOffset += underlineSize; underlineOffset -= textUnderlineOffset.AsLengthPercentage().Resolve(em); } else { + // FIXME underlineOffset = -textUnderlineOffset.AsLengthPercentage().Resolve(em); } } @@ -5496,14 +5508,12 @@ void nsTextFrame::UnionAdditionalOverflow(nsPresContext* aPresContext, params.defaultLineThickness = params.lineSize.height; bool swapUnderline = - verticalDec && !wm.IsSideways() && IsUnderlineRight(this); + verticalDec && !wm.IsSideways() && IsUnderlineRight(*Style()); if (swapUnderline ? lineType == StyleTextDecorationLine::OVERLINE : lineType == StyleTextDecorationLine::UNDERLINE) { - if (dec.mTextUnderlinePosition.IsUnder()) { - params.offset = -metrics.emDescent; - } - SetOffsetIfLength(dec.mTextUnderlineOffset, params, metrics, + SetOffsetIfLength(dec.mTextUnderlinePosition, + dec.mTextUnderlineOffset, params, metrics, appUnitsPerDevUnit, parentWM.IsSideways(), swapUnderline); } @@ -5719,16 +5729,13 @@ void nsTextFrame::DrawSelectionDecorations( aTextPaintStyle.PresContext(), aFontMetrics, aSelectionType); bool swapUnderline = - aVertical && !wm.IsSideways() && IsUnderlineRight(this); + aVertical && !wm.IsSideways() && IsUnderlineRight(*Style()); if (swapUnderline ? aDecoration == StyleTextDecorationLine::OVERLINE : aDecoration == StyleTextDecorationLine::UNDERLINE) { const auto* styleText = StyleText(); - if (styleText->mTextUnderlinePosition.IsUnder()) { - params.offset = -aFontMetrics.emDescent; - } else { - params.offset = aFontMetrics.underlineOffset; - } - SetOffsetIfLength(styleText->mTextUnderlineOffset, params, aFontMetrics, + params.offset = aFontMetrics.underlineOffset; + SetOffsetIfLength(styleText->mTextUnderlinePosition, + styleText->mTextUnderlineOffset, params, aFontMetrics, appUnitsPerDevPixel, wm.IsSideways(), swapUnderline); } else { params.offset = aFontMetrics.maxAscent; @@ -6330,7 +6337,7 @@ void nsTextFrame::PaintTextSelectionDecorations( gfxFont* firstFont = aParams.provider->GetFontGroup()->GetFirstValidFont(); bool verticalRun = mTextRun->IsVertical(); bool useVerticalMetrics = verticalRun && mTextRun->UseCenterBaseline(); - bool rightUnderline = useVerticalMetrics && IsUnderlineRight(this); + bool rightUnderline = useVerticalMetrics && IsUnderlineRight(*Style()); const auto kDecoration = rightUnderline ? StyleTextDecorationLine::OVERLINE : StyleTextDecorationLine::UNDERLINE; gfxFont::Metrics decorationMetrics( @@ -7011,20 +7018,15 @@ void nsTextFrame::DrawTextRunAndDecorations( params.defaultLineThickness = params.lineSize.height; params.baselineOffset = dec.mBaselineOffset / app; - if (lineType == StyleTextDecorationLine::UNDERLINE && - dec.mTextUnderlinePosition.IsUnder()) { - params.offset = -metrics.emDescent; - } else { - params.offset = metrics.*lineOffset; - } + params.offset = metrics.*lineOffset; bool swapUnderline = - verticalDec && !wm.IsSideways() && IsUnderlineRight(this); + verticalDec && !wm.IsSideways() && IsUnderlineRight(*Style()); if (swapUnderline ? lineType == StyleTextDecorationLine::OVERLINE : lineType == StyleTextDecorationLine::UNDERLINE) { - SetOffsetIfLength(dec.mTextUnderlineOffset, params, metrics, - PresContext()->AppUnitsPerDevPixel(), wm.IsSideways(), - swapUnderline); + SetOffsetIfLength(dec.mTextUnderlinePosition, dec.mTextUnderlineOffset, + params, metrics, PresContext()->AppUnitsPerDevPixel(), + wm.IsSideways(), swapUnderline); } SetWidthIfLength(dec.mTextDecorationThickness, ¶ms.lineSize.height, metrics.emHeight, PresContext()->AppUnitsPerDevPixel()); @@ -7317,11 +7319,7 @@ bool nsTextFrame::CombineSelectionUnderlineRect(nsPresContext* aPresContext, nsCSSRendering::DecorationRectParams params; params.ascent = aPresContext->AppUnitsToGfxUnits(mAscent); - if (StyleText()->mTextUnderlinePosition.IsUnder()) { - params.offset = -metrics.emDescent; - } else { - params.offset = fontGroup->GetUnderlineOffset(); - } + params.offset = fontGroup->GetUnderlineOffset(); TextDecorations textDecs; GetTextDecorations(aPresContext, eResolvedColors, textDecs); @@ -7379,9 +7377,10 @@ bool nsTextFrame::CombineSelectionUnderlineRect(nsPresContext* aPresContext, aPresContext->AppUnitsPerDevPixel()); bool swapUnderline = - verticalRun && !wm.IsSideways() && IsUnderlineRight(this); + verticalRun && !wm.IsSideways() && IsUnderlineRight(*Style()); if (swapUnderline ? textDecs.HasOverline() : textDecs.HasUnderline()) { - SetOffsetIfLength(StyleText()->mTextUnderlineOffset, params, metrics, + SetOffsetIfLength(StyleText()->mTextUnderlinePosition, + StyleText()->mTextUnderlineOffset, params, metrics, aPresContext->AppUnitsPerDevPixel(), wm.IsSideways(), swapUnderline); } diff --git a/layout/generic/nsTextFrame.h b/layout/generic/nsTextFrame.h index 00876d84bce8..b3e11c3a3cd4 100644 --- a/layout/generic/nsTextFrame.h +++ b/layout/generic/nsTextFrame.h @@ -862,7 +862,7 @@ class nsTextFrame : public nsFrame { nscoord mBaselineOffset; // This represents the offset from the initial position of the underline - const mozilla::StyleTextDecorationLength mTextUnderlineOffset; + const mozilla::LengthPercentageOrAuto mTextUnderlineOffset; // for CSS property text-decoration-thickness, the width refers to the // thickness of the decoration line @@ -876,7 +876,7 @@ class nsTextFrame : public nsFrame { LineDecoration(nsIFrame* const aFrame, const nscoord aOff, mozilla::StyleTextUnderlinePosition aUnderlinePosition, - const mozilla::StyleTextDecorationLength& aUnderlineOffset, + const mozilla::LengthPercentageOrAuto& aUnderlineOffset, const mozilla::StyleTextDecorationLength& aDecThickness, const nscolor aColor, const uint8_t aStyle) : mFrame(aFrame), diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index e78c534b4e9e..69d6875fab4d 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -3318,7 +3318,7 @@ nsStyleText::nsStyleText(const Document& aDocument) mLetterSpacing({0.}), mLineHeight(StyleLineHeight::Normal()), mTextIndent(LengthPercentage::Zero()), - mTextUnderlineOffset(StyleTextDecorationLength::Auto()), + mTextUnderlineOffset(LengthPercentageOrAuto::Auto()), mTextDecorationSkipInk(StyleTextDecorationSkipInk::Auto), mTextUnderlinePosition(StyleTextUnderlinePosition::AUTO), mWebkitTextStrokeWidth(0), diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index 8df4b7f7f883..c9b41672601b 100644 --- a/layout/style/nsStyleStruct.h +++ b/layout/style/nsStyleStruct.h @@ -1149,7 +1149,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleText { mozilla::StyleLineHeight mLineHeight; mozilla::LengthPercentage mTextIndent; - mozilla::StyleTextDecorationLength mTextUnderlineOffset; + mozilla::LengthPercentageOrAuto mTextUnderlineOffset; mozilla::StyleTextDecorationSkipInk mTextDecorationSkipInk; mozilla::StyleTextUnderlinePosition mTextUnderlinePosition; @@ -1277,7 +1277,14 @@ inline StyleTextTransform StyleTextTransform::None() { inline bool StyleTextTransform::IsNone() const { return *this == None(); } -inline bool StyleTextUnderlinePosition::IsAuto() const { return *this == AUTO; } +// Note that IsAuto() does not exclude the possibility that `left` or `right` +// is set; it refers only to behavior in horizontal typographic mode. +inline bool StyleTextUnderlinePosition::IsAuto() const { + return !(*this & (StyleTextUnderlinePosition::FROM_FONT | StyleTextUnderlinePosition::UNDER)); +} +inline bool StyleTextUnderlinePosition::IsFromFont() const { + return bool(*this & StyleTextUnderlinePosition::FROM_FONT); +} inline bool StyleTextUnderlinePosition::IsUnder() const { return bool(*this & StyleTextUnderlinePosition::UNDER); } diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index c9a9abdd3b5e..d7945370d5dd 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -7688,17 +7688,15 @@ var gCSSProperties = { applies_to_first_line: true, applies_to_placeholder: true, initial_values: ["auto"], - other_values: [ + other_values: ["0", "-14px", "25px", "100em", "-45em", "43%", "-10%"], + invalid_values: [ + "13", + "-25", + "rubbish", + ",./!@#$", "from-font", - "0", - "-14px", - "25px", - "100em", - "-45em", - "43%", - "-10%", + "from font", ], - invalid_values: ["13", "-25", "rubbish", ",./!@#$", "from font"], }, "text-underline-position": { domProp: "textUnderlinePosition", @@ -7716,16 +7714,23 @@ var gCSSProperties = { "under left", "right under", "under right", + "from-font", + "from-font left", + "from-font right", + "left from-font", + "right from-font", ], invalid_values: [ "none", + "auto from-font", "auto under", + "under from-font", "left right", "right auto", "0", "1px", "10%", - "from-font", + "from font", ], }, "text-emphasis": { diff --git a/servo/components/style/properties/longhands/inherited_text.mako.rs b/servo/components/style/properties/longhands/inherited_text.mako.rs index 9784e2529e92..f01bedb177ef 100644 --- a/servo/components/style/properties/longhands/inherited_text.mako.rs +++ b/servo/components/style/properties/longhands/inherited_text.mako.rs @@ -382,8 +382,8 @@ ${helpers.single_keyword( // text underline offset ${helpers.predefined_type( "text-underline-offset", - "TextDecorationLength", - "generics::text::GenericTextDecorationLength::Auto", + "LengthPercentageOrAuto", + "computed::LengthPercentageOrAuto::auto()", engines="gecko", animation_value_type="ComputedValue", gecko_pref="layout.css.text-underline-offset.enabled", diff --git a/servo/components/style/values/computed/text.rs b/servo/components/style/values/computed/text.rs index 57a142860d95..0ca2e6044ed1 100644 --- a/servo/components/style/values/computed/text.rs +++ b/servo/components/style/values/computed/text.rs @@ -27,7 +27,7 @@ pub use crate::values::specified::{TextDecorationSkipInk, TextTransform}; /// A computed value for the `initial-letter` property. pub type InitialLetter = GenericInitialLetter; -/// Implements type for `text-underline-offset` and `text-decoration-thickness` properties +/// Implements type for `text-decoration-thickness` property. pub type TextDecorationLength = GenericTextDecorationLength; /// A computed value for the `letter-spacing` property. diff --git a/servo/components/style/values/generics/text.rs b/servo/components/style/values/generics/text.rs index 6ad983e00f99..ceeb59a3d0ce 100644 --- a/servo/components/style/values/generics/text.rs +++ b/servo/components/style/values/generics/text.rs @@ -123,8 +123,8 @@ impl LineHeight { } } -/// Implements type for text-underline-offset and text-decoration-thickness -/// which take the grammar of auto | from-font | | +/// Implements type for text-decoration-thickness +/// which takes the grammar of auto | from-font | | /// /// https://drafts.csswg.org/css-text-decor-4/ #[repr(C, u8)] diff --git a/servo/components/style/values/specified/text.rs b/servo/components/style/values/specified/text.rs index 835fefa292a1..438b926802f5 100644 --- a/servo/components/style/values/specified/text.rs +++ b/servo/components/style/values/specified/text.rs @@ -1029,7 +1029,7 @@ pub enum TextDecorationSkipInk { None, } -/// Implements type for `text-underline-offset` and `text-decoration-thickness` properties +/// Implements type for `text-decoration-thickness` property pub type TextDecorationLength = GenericTextDecorationLength; impl TextDecorationLength { @@ -1048,21 +1048,23 @@ impl TextDecorationLength { bitflags! { #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem)] - #[value_info(other_values = "auto,under,left,right")] + #[value_info(other_values = "auto,from-font,under,left,right")] #[repr(C)] /// Specified keyword values for the text-underline-position property. - /// (Non-exclusive, but not all combinations are allowed: only `under` may occur - /// together with either `left` or `right`.) - /// https://drafts.csswg.org/css-text-decor-3/#text-underline-position-property + /// (Non-exclusive, but not all combinations are allowed: the spec grammar gives + /// `auto | [ from-font | under ] || [ left | right ]`.) + /// https://drafts.csswg.org/css-text-decor-4/#text-underline-position-property pub struct TextUnderlinePosition: u8 { /// Use automatic positioning below the alphabetic baseline. const AUTO = 0; + /// Use underline position from the first available font. + const FROM_FONT = 1 << 0; /// Below the glyph box. - const UNDER = 1 << 0; + const UNDER = 1 << 1; /// In vertical mode, place to the left of the text. - const LEFT = 1 << 1; + const LEFT = 1 << 2; /// In vertical mode, place to the right of the text. - const RIGHT = 1 << 2; + const RIGHT = 1 << 3; } } @@ -1085,7 +1087,12 @@ impl Parse for TextUnderlinePosition { "auto" if result.is_empty() => { return Ok(result); }, - "under" if !result.intersects(TextUnderlinePosition::UNDER) => { + "from-font" if !result.intersects(TextUnderlinePosition::FROM_FONT | + TextUnderlinePosition::UNDER) => { + result.insert(TextUnderlinePosition::FROM_FONT); + }, + "under" if !result.intersects(TextUnderlinePosition::FROM_FONT | + TextUnderlinePosition::UNDER) => { result.insert(TextUnderlinePosition::UNDER); }, "left" if !result.intersects(TextUnderlinePosition::LEFT | @@ -1131,6 +1138,7 @@ impl ToCss for TextUnderlinePosition { }; } + maybe_write!(FROM_FONT => "from-font"); maybe_write!(UNDER => "under"); maybe_write!(LEFT => "left"); maybe_write!(RIGHT => "right"); diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml index 2127de20ae65..5393fcf8263e 100644 --- a/servo/ports/geckolib/cbindgen.toml +++ b/servo/ports/geckolib/cbindgen.toml @@ -398,6 +398,7 @@ renaming_overrides_prefixing = true "TextUnderlinePosition" = """ inline bool IsAuto() const; + inline bool IsFromFont() const; inline bool IsUnder() const; inline bool IsLeft() const; inline bool IsRight() const; diff --git a/testing/web-platform/tests/css/css-text-decor/text-underline-offset-computed.html b/testing/web-platform/tests/css/css-text-decor/text-underline-offset-computed.html index 32dfd24fbaa5..26861c1565bc 100644 --- a/testing/web-platform/tests/css/css-text-decor/text-underline-offset-computed.html +++ b/testing/web-platform/tests/css/css-text-decor/text-underline-offset-computed.html @@ -13,7 +13,6 @@
diff --git a/testing/web-platform/tests/css/css-text-decor/text-underline-offset-invalid.html b/testing/web-platform/tests/css/css-text-decor/text-underline-offset-invalid.html index 6d96616a829c..9f77be0359f4 100644 --- a/testing/web-platform/tests/css/css-text-decor/text-underline-offset-invalid.html +++ b/testing/web-platform/tests/css/css-text-decor/text-underline-offset-invalid.html @@ -4,13 +4,14 @@ CSS Text Decoration Test: parsing text-underline-offset with invalid values - + @@ -12,7 +12,6 @@