1453298 - percentage text-indent should be relative to the containing block of the text, not the containing block of the block

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, enhancement, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In https://github.com/w3c/csswg-drafts/issues/2394 the CSS WG is resolving that percentage text-indent should be resolved against the containing block of the text (or the content width of the element).  We need to change our behavior.  (The old spec text could be read in two different ways, and it actually led to implementation divergence.)

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I believe this makes bug 908706 and bug 1306590 invalid, since I think they're both about fixing bugs with the old now-incorrect behavior.

Comment 3

[Mats Palmgren (inactive)]

I've filed a chromium bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=884588

I'd strongly recommend that we *don't* ship this change before
Chrome/Safari does.  All UAs are currently (mostly) compatible
in using the containing block's inline size to resolve text-indent,
so there's a significant web-compat risk here.

Comment 4

[Mats Palmgren (inactive)]

It's a trivial change though:
https://hg.mozilla.org/try/rev/bb72a7a112ca6c29747e42d035a05fa14ecd4dc3

Comment 5

[Mats Palmgren (inactive)]

It appears Chrome has landed a fix for this now.

Comment 6

[Mats Palmgren (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cde9790d54d29635f10aee9d094480908f4cdc0

Comment 7

[Mats Palmgren (inactive)]

Attachment: [css-text] Resolve 'text-indent' percentage against the content-box inline-size of the box itself, not its containing block

A couple of notes on the test changes:
The test files css/css-text/text-indent/text-indent-percentage-*.html
placed two "X" on top of each other, one of which had red color.
This is bad because it produces different colored AA pixels compared
to the reference, which is just a black "X".  Also, there's no need
for this red reference "X" in the test itself since the reference
shows the expected rendering anyway.

The added fuzz factor on Android for 1377447-1.html is unrelated to
this bug, but it annoyed me to it fail all the time on my Try pushes.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9019371 [details] [diff] [review]
[css-text] Resolve 'text-indent' percentage against the content-box inline-size of the box itself, not its containing block

Review of attachment 9019371 [details] [diff] [review]:
-----------------------------------------------------------------

I'm a bit confused about the body { background: white } bits, but assuming it has a reasonable explanation for why are they needed, or they get removed before landing, this looks good to me.

::: testing/web-platform/tests/css/css-text/text-indent/text-indent-percentage-001.xht
@@ +17,5 @@
>  			}
>  			#reference1
>  			{
>  				color: red;
> +				left: 100px; /* see comments for #test1 below */

Thanks for fixing these! I'll mention it in https://github.com/web-platform-tests/wpt/issues/13581 so that nobody duplicates work.

::: testing/web-platform/tests/css/css-text/text-indent/text-indent-percentage-002.html
@@ +7,5 @@
>  <meta name="flags" content="">
>  <link rel="match" href="reference/text-indent-percentage-002-ref.html">
>  <meta name="assert" content="Percentages in text-indent refer to width of the element's content box">
>  <style>
> +body { background: white; }

Hmm, is this line really needed? Isn't the default canvas color white?

Comment 9

[Mats Palmgren (inactive)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)
> > +body { background: white; }
> 
> Hmm, is this line really needed? Isn't the default canvas color white?

The tests has "border-right: 10px solid white" which makes the assumption
that the canvas is by default white.   It's pedantic, but there's no spec
that requires that normatively, AFAIK.

(curiosa: NCSA Mosaic and early versions of Netscape/IE browsers didn't
have a white canvas.  I think it was the native platform's "window"
background color. Typically grey, but I think it varied with the theme.)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2775681c72a3
[css-text] Resolve 'text-indent' percentage against the content-box inline-size of the box itself, not its containing block.  r=emilio

Comment 11

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

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

Comment 12

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

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/13721
* continuous-integration/appveyor/pr
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/AAHMlX0kQlq9SOAtDbP1DQ)

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/2775681c72a3

Comment 14

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

Upstream PR merged

Comment 15

[Kohei Yoshino [:kohei]]

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/percentage-on-text-indent-is-now-relative-to-containing-block-instead-of-the-parent/