line-height quirks should probably not apply inside of generated content
Categories
(Core :: Layout: Block and Inline, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox78 | --- | affected |
People
(Reporter: dbaron, Unassigned)
Details
Attachments
(5 files)
jdm pasted a testcase into the layout channel on matrix where Gecko produces different results from Chromium.
I think the underlying issue here is that we're applying the line-height quirk to generated content, and Chromium is not. I'll attach a few testcases.
|
Reporter | |
Comment 1•5 years ago
|
||
|
|
Reporter | |
Comment 2•5 years ago
|
||
|
|
Reporter | |
Comment 3•5 years ago
|
||
|
|
Reporter | |
Comment 4•5 years ago
|
||
Oops, the descriptive text for the hashless hex color quirk is wrong in all three examples: it's present only in quirks mode.
|
|
Reporter | |
Comment 5•5 years ago
|
||
One interesting question about the description of the line-height quirk how it should apply to inlines that are split across lines. So far I haven't been able to write a testcase where the line-height quirk has any effect at all on inlines that are split across lines. However, this is a testcase that I was hoping might be such a testcase, but instead it's a testcase where Gecko and Chromium disagree... but where they're both internally consistent across all three modes.
|
||
Comment 6•5 years ago
|
||
This is a bug in Chromium, not in Gecko. The spec says "an inline box that matches the following conditions ..." and there's no condition that says that inline boxes for pseudo-elements should be excluded. I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1080904
|
|
Reporter | |
Comment 7•5 years ago
|
||
If you take literally the way the spec is worded, it doesn't apply to any inline boxes that cross more than one line, which this one does. That's what I was trying to probe at in comment 6.
|
|
Reporter | |
Comment 8•5 years ago
|
||
... or at the very least, if it does apply, it applies to the entire box rather than separately for the fragment on each line. I'm reasonably confident that whatever we do looks at only the fragment on the current line.
|
|
Reporter | |
Comment 9•5 years ago
|
||
(Also, don't take the quirks mode spec too literally as what we should do -- it was reverse-engineered based on what we do do, and not necessarily with a lot of attention to detailed interactions or interesting cases.)
|
|
||
Comment 10•5 years ago
|
||
I mistakenly thought Chrome made a difference between generated content and not, but they treat all these the same. So yeah, I think this is probably a Gecko bug after all. (I've closed the chromium bug.)
|
|
||
Comment 11•5 years ago
|
||
The problem seems to be in calculating zeroEffectiveSpanBox here:
https://searchfox.org/mozilla-central/rev/e9131060a0322f6c181b4933ac74e6440b6e723d/layout/generic/nsLineLayout.cpp#1837
we use preMode which is set here:
https://searchfox.org/mozilla-central/rev/e9131060a0322f6c181b4933ac74e6440b6e723d/layout/generic/nsLineLayout.cpp#1747
where mStyleText is the style for the block. Checking pfd->mIsNonWhitespaceTextFrame (for a text frame) together with the block's style make no sense to me - we should query the text frame's own style data instead of preMode, i.e. pfd->mFrame->StyleText()->WhiteSpaceIsSignificant().
Note that the same check is made in nsTextFrame::IsEmpty() and here we use the text frame's own style:
https://searchfox.org/mozilla-central/rev/e9131060a0322f6c181b4933ac74e6440b6e723d/layout/generic/nsTextFrame.cpp#9936-9937
Let's see what breaks:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1f9c9be88039e5ede136ade99e30513eb970acf
|
|
||
Comment 12•5 years ago
|
||
We should probably audit the correctness of the other uses of preMode in this function too:
https://searchfox.org/mozilla-central/rev/e9131060a0322f6c181b4933ac74e6440b6e723d/layout/generic/nsLineLayout.cpp#2221
https://searchfox.org/mozilla-central/rev/e9131060a0322f6c181b4933ac74e6440b6e723d/layout/generic/nsLineLayout.cpp#2280
|
|
||
Comment 13•5 years ago
|
||
I might as well take this since I'm working on a fix...
BTW, there are tests for this in testing/web-platform/tests/quirks/line-height-calculation.html (which we currently fail).
|
||
Comment 14•3 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Description
•