1600224 - Unexpectedly small line box, with "display:contents; line-height:[anything]"

Status: NEW

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

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Load attached testcase.

EXPECTED RESULTS:
There should be a box around the text.

ACTUAL RESULTS:
The box is very tiny and doesn't fit the text. It seems to be as tall as the inherited font-size (which in this case is tiny).

Note that the testcase includes a custom line-height value, which Chrome honors (and I'm not sure if it makes sense to honor or not, but we probably should?). Instead of honoring it, we throw it away and then trigger this bug. It does have some effect, though, because if I remove the line-height entirely, then we produce EXPECTED RESULTS with a box that's as tall as the text.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: semi-reference case 1 (removed `line-height`)

Here's a reference-ish case (not an exact reference, but just for comparison). It's identical to the testcase, except I've removed line-height:50px which we're not respecting anyway.

With that removal, we produce a nicely sized box and nicely positioned text, for some reason.

Comment 2

[Daniel Holbert [:dholbert]]

(This might really be a style-system bug -- not sure.)

Comment 3

[Daniel Holbert [:dholbert]]

Meant to say:

• Chrome and Safari give EXPECTED RESULTS.
• this bug is associated with webcompat issue https://github.com/webcompat/web-bugs/issues/45375 ( https://webcompat.com/issues/45375 )

Comment 4

[Emilio Cobos Álvarez (:emilio)]

So the issue here seems to be that we look at the wrong font-size during line layout... I'm ~sure there's no style system bug but I can take a look anyway.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Here's the bug: https://searchfox.org/mozilla-central/rev/42c2ecdc429115c32e6bcb78bf087a228a051044/layout/generic/nsLineLayout.cpp#2202

I don't understand why... But we should probably do that for display: contents children.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1600224 - Allow text frames to affect line height when their line-height is different than normal.

I don't get why this wouldn't be the right thing to do.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Let's see what try thinks: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d92002c79fd588d18c0172773f69d04e7aba615

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Well, that was unexpectedly super-orange... :/

It seems like Blink and WebKit create an anonymous inline around display: contents text: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/layout_tree_builder.cc?l=113&rcl=b6b46648babb128b40277a4bdcf7a1bde09ba81d

Jonathan, do you have any insight here? Is there anything totally off with the patch in comment 6?

Comment 10

[Dennis Schubert [:denschub]]

No currently known WebCompat breakage for this, unsetting the flag accordingly.