1979915 - Implement rendering support for CSS text-decoration-trim

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, enhancement)

Description

[Jonathan Kew [:jfkthame]]

We need to take the text-decoration-trim inset/outset amounts into account when computing decoration line rects.

There might be some trickiness regarding what are the actual ends of the decoration line, as opposed to element/frame boundaries that fall within the scope of a continuous line (and so trimming doesn't apply).

Comment 1

[Sebastian Zartner [:sebo]]

This needs to be documented once implemented. (And it's also worth a release note then.)

Sebastian

Comment 2

[Emily McDonough [:alaskanemily]]

Attachment: Bug 1979915 Part 1 - Add helper to create a zeroed StyleTextDecorationTrim

Comment 3

[Emily McDonough [:alaskanemily]]

Attachment: Bug 1979915 Part 2 - Consider text-decoration-trim for text decoration rect calculation

Comment 4

[Jonathan Kew [:jfkthame]]

I've been thinking a bit more about what this needs to do. With regard to nested inlines within the scope of a decoration, it's not sufficient to detect that the frame currently being painted isn't the one where the decoration was defined, or that it's "internal" to the decoration range, and therefore omit any trimming. The decoration line for a sub-element might still need to be partially trimmed, but not necessarily by the full amount specified by the property; it depends how close it is to the edge of the complete range where the decoration applies.

Consider something like <u>foo <b>the quick brown fox</b> bar</u>, with a positive text-decoration-trim applied to the <u> element. When we're rendering the textframe for <b>, we don't want to blindly apply the trim amount, because that will break the underline. But suppose the trim amount is larger than the width of "foo "; in that case we do need to apply some trimming at the beginning of the <b> subelement, but only the remaining amount of trim, after accounting for the width that "foo " has absorbed.

One way I think we can handle this is when setting up the parameters in nsTextFrame::DrawTextRunAndDecorations. Rather than passing the StyleTextDecorationTrim value in the parameters (for nsCSSRendering code to resolve), we can compare the rect of the current frame with that of the frame where the decoration was defined (using nsIFrame::GetOffsetTo), and determine if there is any "remaining" trim inset that needs to be applied to this part of the line. So the parameters will have trimStart and trimEnd values that reflect the amount of trim to apply to this segment, not the original values from the style.

(If the trim values are negative -- i.e. extending the line -- then they should just be set to zero for "internal" segments that are not at the edges of the original decorating rect. It's positive values -- trimming the line -- that need to be more carefully tracked in case internal segments have some residual trim to apply.)

Comment 5

[Jonathan Kew [:jfkthame]]

Another observation relates to the failure to render "extended" decoration lines (when the trim value is negative). I tried hacking nsTextFrame::UnionAdditionalOverflow to extend the ink-overflow rect in the inline direction when negative trim is present, but this does not result in the extended part of the line getting rendered.

It may still be necessary to update the ink-overflow area in this way for reliable rendering, but it appears it is not sufficient; it looks like there must also be code elsewhere in the rendering path that is still constraining or clipping the decoration line to the width of the text frame.

Comment 6

[Jonathan Kew [:jfkthame]]

Ah, see https://searchfox.org/firefox-main/rev/d0ff31da7cb418d2d86b0d83fecd7114395e5d46/layout/generic/nsTextFrame.cpp#7445-7452. We'll need to disable clipping here for edges of decoration lines that are supposed to have negative trim applied (or at least, extend the clip area by the trim amount in that direction).

Comment 7

[Emily McDonough [:alaskanemily]]

Attachment: Bug 1979915 WIP - Use text-decoration-trim for text decoration rect calculation and decoration painting

Comment 8

[Emily McDonough [:alaskanemily]]

(In reply to Jonathan Kew [:jfkthame] from comment #6)

Ah, see https://searchfox.org/firefox-main/rev/d0ff31da7cb418d2d86b0d83fecd7114395e5d46/layout/generic/nsTextFrame.cpp#7445-7452. We'll need to disable clipping here for edges of decoration lines that are supposed to have negative trim applied (or at least, extend the clip area by the trim amount in that direction).

As a first pass, I think just disabling it for a negative trim seems simple enough. We'll definitely want to properly extend the clipping area as a followup though.

Comment 9

[Phabricator Automation]

Comment on attachment 9513347 [details]
Bug 1979915 WIP - Use text-decoration-trim for text decoration rect calculation and decoration painting

Revision D264878 was moved to bug 1988793. Setting attachment 9513347 [details] to obsolete.

Comment 10

[Emily McDonough [:alaskanemily]]

Attachment: Bug 1979915 Part 1 - Apply text-decoration-trim during decoration rect calculation and decoration painting.

Comment 11

[Emily McDonough [:alaskanemily]]

Attachment: Bug 1979915 Part 2 - Update text expectations for implementing text-decoration-trim

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/05067d0cbe1e
https://hg.mozilla.org/integration/autoland/rev/2ede4e4f6d6b
Part 1 - Apply text-decoration-trim during decoration rect calculation and decoration painting. r=layout-reviewers,layout-jp-market-reviewers,jfkthame
https://github.com/mozilla-firefox/firefox/commit/b52edf65bf2f
https://hg.mozilla.org/integration/autoland/rev/070ea0ba4d20
Part 2 - Update text expectations for implementing text-decoration-trim r=layout-jp-market-reviewers,jfkthame,TYLin

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/160d4f9b3114
https://hg.mozilla.org/integration/autoland/rev/069b3c837fda
Revert "Bug 1979915 Part 2 - Update text expectations for implementing text-decoration-trim r=layout-jp-market-reviewers,jfkthame,TYLin" for causing Wr failures in text-decoration-trim-007.html

Comment 14

[Cristian Tuns]

Backed out for causing wr failures

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | /css/css-text-decor/text-decoration-trim-007.html | Testing http://web-platform.test:8000/css/css-text-decor/text-decoration-trim-007.html == http://web-platform.test:8000/css/css-text-decor/reference/text-decoration-trim-007-ref.html

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1c6a8b56e6f5
https://hg.mozilla.org/mozilla-central/rev/e8a13246f9aa
Revert "Bug 1979915 Part 2 - Update text expectations for implementing text-decoration-trim r=layout-jp-market-reviewers,jfkthame,TYLin" for causing Wr failures in text-decoration-trim-007.html

Comment 16

[Emily McDonough [:alaskanemily]]

This failure is caused by some fonts only sometimes having skip-ink (even given the text decoration offset), and the reference case using an invisible copy of the text to shift the decoration. The skip-ink intercepts end up shifted too, which misaligns them.
I've disabled skip-ink for those tests, we already have multiple WPT that test trim values (positive and negative) with skip-ink.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/713a02b2fa62
https://hg.mozilla.org/integration/autoland/rev/c39ae4580d11
Part 1 - Apply text-decoration-trim during decoration rect calculation and decoration painting. r=layout-reviewers,layout-jp-market-reviewers,jfkthame
https://github.com/mozilla-firefox/firefox/commit/9ad061062c44
https://hg.mozilla.org/integration/autoland/rev/5013885993a2
Part 2 - Update text expectations for implementing text-decoration-trim r=layout-jp-market-reviewers,jfkthame,TYLin

Comment 18

[Sebastian Zartner [:sebo]]

[Why is this notable]: New CSS property allowing authors to manipulate the length of text decorations.
[Affects Firefox for Android]: yes
[Suggested wording]: Starting with Firefox x, the text-decoration-trim property is supported, which allows authors to adjust the start and end points of line decorations.
[Links (documentation, blog post, etc)]:
https://drafts.csswg.org/css-text-decor-4/#text-decoration-skip-inset-property
https://developer.mozilla.org/en-US/docs/Web/CSS/text-decoration-trim (doesn't exist yet)

Comment 19

[Emily McDonough [:alaskanemily]]

See intent to ship notice: https://groups.google.com/a/mozilla.org/g/dev-platform/c/YrBMtBkbZYo
This won't be enabled until 146.

Comment 20

[Sebastian Zartner [:sebo]]

I know, the request in comment 18 is for the Nightly release notes.

Sebastian

Comment 21

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/c39ae4580d11
https://hg.mozilla.org/mozilla-central/rev/5013885993a2

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/55303 for changes under testing/web-platform/tests

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 24

[Chris Mills [:cmills]]

Hi all. I'm looking at documenting this on MDN, but I'm not convinced that it is ready to document yet. It isn't implemented in Chromium/WebKit yet, and in Fx it seems quite broken:

• When you apply it to an inline element, it works as expected.
• When you apply it to a block-level element, the end offset doesn't work at all.
• When you center the text using text-align, it completely stops the property from working.

The code I used to test it is available at https://codepen.io/Chris-Mills/pen/XJXBpmp.

Comment 25

[Jonathan Kew [:jfkthame]]

Were you using latest Nightly to test? The issues mentioned sound like things that should have been fixed by bug 1993939, which just landed yesterday.

Comment 26

[Chris Mills [:cmills]]

(In reply to Jonathan Kew [:jfkthame] from comment #25)

Were you using latest Nightly to test? The issues mentioned sound like things that should have been fixed by bug 1993939, which just landed yesterday.

Ah, I first saw these issues last week, and had forgotten to update Nightly before testing again. All good now; I'll get on and document it!

Comment 27

[Jonathan Kew [:jfkthame]]

One thing to note, though -- the CSS working group is discussing whether to rename the property (if so, most likely to text-decoration-inset, I think). So it may make sense to hold off on publishing anything until that decision is made. I'm hoping they'll come to a resolution very soon (perhaps at this week's meeting).

Comment 28

[Chris Mills [:cmills]]

(In reply to Jonathan Kew [:jfkthame] from comment #27)

One thing to note, though -- the CSS working group is discussing whether to rename the property (if so, most likely to text-decoration-inset, I think). So it may make sense to hold off on publishing anything until that decision is made. I'm hoping they'll come to a resolution very soon (perhaps at this week's meeting).

Good to know, thanks. I'll hold off for now.

Comment 29

[Sebastian Zartner [:sebo]]

(In reply to Jonathan Kew [:jfkthame] from comment #27)

One thing to note, though -- the CSS working group is discussing whether to rename the property (if so, most likely to text-decoration-inset, I think). So it may make sense to hold off on publishing anything until that decision is made. I'm hoping they'll come to a resolution very soon (perhaps at this week's meeting).

And just for reference, this is discussed in https://github.com/w3c/csswg-drafts/issues/8402.

Sebastian

Comment 30

[Dianna Smith [:diannaS]]

this is mentioned in 146 release notes via bug 1993043

Comment 31

[Chris Mills [:cmills]]

See https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/text-decoration-inset.