1646639 - Bug in rendering CSS ::first-line

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Reiner Makohl]

Attachment: test-first-line.html / source code shows steps to reproduce the results

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:77.0) Gecko/20100101 Firefox/77.0

Steps to reproduce:

1. I have defined the style ::first-line.
2. I would like to format the first character in the sentence (p-block) with an additional style statement in which the style statement "float:left" occurs ("<p><span style="float:left; ...").

That works very well! (as in other browsers)

3. But if I use a Hebrew character set in any other place in the same document, Firefox will incorrectly render the span-Element.

Actual results:

In this case Firefox doubles the actual space the character needs inside the span-box when rendering the output (depends on charcter width).

This moves the rest of the lines of the p-block to the right.
The output also overlaps the boundaries of an enclosing div-box.

However, the debugger shows correct values for all elements (as defined in html) and does not "see" the doubled space, not even the incorrect positioning of the p-block.

In order not to have to describe it in a lengthy way, I have included a simple html example (with comments) in which the different results can be easily generated and observed. -> Please check source code and modify to see the results.

Expected results:

Same output with/with out using somewhere in the doc Hebrew characters (as in other browsers).

(Not tested with other country-specific character sets.)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:alaskanemily, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Emily McDonough [:alaskanemily]]

I can confirm this occurs. @jfkthame: Do you know what is happening here? It seems as though we are measuring the 'C' to be the width that it pushes the rest of the text over, although a float: left; on the rest of the line's text does appear to push it back against the 'C'.

Comment 4

[Jonathan Kew [:jfkthame]]

Wow, that's really weird. Not sure what's happening here.

I can reproduce even with a simplified example:

data:text/html;charset=utf-8,<style>::first-line{}</style><p><span style="float:left;">C</span>heck this line!</p><p>%D7%90</p>

Without the Hebrew character, the problem doesn't occur. Without the (empty) ::first-line rule, it doesn't occur.

However, note that if you modify the existing document in the browser to delete the Hebrew character, the problem does not go away. This will presumably be because the bidi-processing flag has been set on the document, and once it's set, it doesn't get cleared.

So in some way the combination of bidi processing and ::first-line is leading to the error.

A further issue is that we seem to mishandle padding when applied to the floated span:

data:text/html;charset=utf-8,<style>::first-line{}</style><p><span style="float:left;padding-left:10px;background:yellow">C</span>heck this line!</p><p>%D7%90</p>

Here, the "C" is painted too far to the right; compare the position of the yellow background.

Looks like this is a very long-standing bug, it's not a regression (at least not a recent one).

Comment 5

[Jonathan Kew [:jfkthame]]

Slightly different mis-rendering, but probably a related underlying bug: here's an example that doesn't involve float at all:

data:text/html;charset=utf-8,<style>::first-line{}</style><p><span style="display:inline-block;padding-left:10px;background:yellow">C</span>heck this line!</p><p>%D7%90</p>

Comment 6

[Jonathan Kew [:jfkthame]]

And one more example: observe that the size of the abs-pos <div> is ignorant of the misplaced content within it:

data:text/html;charset=utf-8,<style>::first-line{}</style><div style="position:absolute;border:1px solid gray"><p><span style="float:left;padding-left:20px;background:yellow">C</span> this line!</p><p>%D7%90</p>

Comment 7

[Jonathan Kew [:jfkthame]]

Turns out this has been noticed/filed before in bug 1495674, and there's even a (currently-failing) WPT testcase for it. I'm going to forward-dupe the older bug to this one, as we have a selection of testcases here that all exhibit the same underlying bug.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: More extensive testcase showing lots of failures

It turns out that our ::first-line rendering is pretty broken for bidi-enabled documents: in general we get things wrong if the first line needs to be adjusted to account for any kind of indentation/padding at the inline-start side, or for text-align adjustment.

This testcase tries a variety of combinations, and of the 36 examples, by my count only 6 render correctly. In the remaining 30 cases, either the green first-line text is misplaced, or its pink background is misplaced -- or in some cases, both the text and background are misplaced but in different ways. There are even a couple of cases where the text on the second line is also affected.

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1646639 - Don't recursively apply the line's start-offset and text-indent to children of a first-line frame. r=dholbert

But do apply any adjustment needed for text-align to the first-line frame itself.

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1646639 - Add collection of testcases here as a WPT reftest. r=dholbert

We get a small amount of antialiasing-related "fuzz" on some platforms,
so those are noted in the corresponding .ini files.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5caaf51dac99
Don't recursively apply the line's start-offset and text-indent to children of a first-line frame. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/e1d587422484
Add collection of testcases here as a WPT reftest. r=dholbert

Comment 13

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

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

Comment 14

[Razvan Maries]

Backed out for perma failures.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=XUbtiq_kQayNVVPbD9fxOA.0&resultStatus=testfailed%2Cbusted%2Cexception&revision=60b6915e80372d04839514e1cef668c9e311e21a&searchStr=windows%2C10%2Cx64%2Cdebug%2Cweb%2Cplatform%2Ctests%2Ctest-windows10-64%2Fdebug-web-platform-tests-reftest-e10s-1%2Cw%28wr1%29

Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=309461771&repo=autoland&lineNumber=3978

Backout: https://hg.mozilla.org/integration/autoland/rev/5ac9dd38a22a404ea82ecec30e2c13d9ec11d0a4

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df21126513e5
Don't recursively apply the line's start-offset and text-indent to children of a first-line frame. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/60c0b51986c6
Add collection of testcases here as a WPT reftest. r=dholbert

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/df21126513e5
https://hg.mozilla.org/mozilla-central/rev/60c0b51986c6

Comment 17

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

Upstream PR merged by moz-wptsync-bot