1435385 - Firefox disagrees with Chrome/Edge on vertical placement of empty "display: list-item" elements (causing misrendering on loecsen.com after we stop honoring their @-moz-document hackaround)

Status: RESOLVED WORKSFORME

(Web Compatibility :: Desktop, defect, P1)

Description

[Markus Popp]

Attachment: Rendering in Firefox 58

I found a significant difference in how the pages at loecsen.com are rendered between Firefox 58 and 59.

On pages like e.g. https://www.loecsen.com/en/learn-german one can see that items on the left are out of line in Firefox 59 (and 60 as well).

This seems to be caused by the way items are inherited. In Firefox 58 there is a weight: 40px setting which is missing in Firefox 59 and it seems like there is an entire "Inherited from a" section is Firefox 59 which is missing in Firefox 58.

The attached screenshots should show clearly what I mean.

Comment 1

[Markus Popp]

Attachment: Rendering in Firefox 59

Comment 2

[Timothy Nikkel (:tnikkel)]

Regression window:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4ad2150e73c0b19f1f6264a4b9ad6927986d43ce&tochange=37e0bd919af057d44c5c1410458c0f00a3653c11

Comment 3

[Markus Popp]

(In reply to Markus Popp from comment #0)
> This seems to be caused by the way items are inherited. In Firefox 58 there
> is a weight: 40px setting which is missing in Firefox 59 and it seems like

weight: 40px should be height: 40px of course.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: testcase

Thanks for the report :)

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: More self-descriptive test-case.

So it seems that Blink / WebKit chose to make the transform origin the <li> element instead of the <a> element, for some reason I don't understand.

However they _do_ chose to use the <a> element (which is an empty, relatively-positioned block) to be the origin of the transform if there's at least an in-flow kid that isn't whitespace.

Something here smells fishy, and I can't test Edge...

Comment 6

[Emilio Cobos Álvarez (:emilio)]

So, Markus pointed out that even without the transform the positioning is different, so probably the transform itself isn't an issue here, and it's a layout bug.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

(or difference. In particular, Blink's behavior looks somewhat borked / inconsistent to me off-hand)

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Even more simplified test-case.

So, if you comment out the display: list-item declaration, suddenly Blink agrees with Firefox re. where to position the anchor in all cases... I have no clue what spec could justify that...

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8948037 [details]
Even more simplified test-case.

I should learn to just attach the file instead of copy-pasting it...

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Seems like edge agrees with Chrome / WebKit, fascinating... Also, adding list-style-type: none makes other browsers agree with us... So I'll dig a bit more tomorrow

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: testcase with empty vs. nonempty div (orange line or box) inside of container div of various flavors

Here's another testcase, with the text removed (and some descriptive text added).

Chrome (and probably other browsers) render *specifically* the example with only "display:list-item" and zero text with the orange line in a different spot (at the bottom of the line-box, rather than at the top.

That impacts the earlier testcases, too, because there, the orange box has an abspos child (which is effectively like the orange box being empty), and the static position is set to be the top-left corner of the orange box.  Chrome's lower position for the orange box in this special scenario is what's causing the difference here.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

https://searchfox.org/mozilla-central/rev/e06af9c36a73a27864302cd2f829e6200dee8541/layout/generic/nsBlockFrame.cpp#1284 looks suspicious...

Comment 13

[Daniel Holbert [:dholbert]]

I think emilio already discovered this, but just to be clear -- the "rendering difference between Firefox 58 and 59" here (on the original page) is simply that we stopped supporting @-moz-document rules.  And the original page was relying on a @-moz-document rule to provide some Firefox-specific CSS, to work around a case where Firefox differs from other browsers.
> @-moz-document url-prefix(){#scroller ul a{height:40px}}

Without that CSS being honored, we get the same CSS as everyone else, which we render differently (as shown by the attached testcases here).

--> Clarifying summary to describe the root issue here.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

So it's still unclear to me what should happen here. Just some notes so far.

So in https://drafts.csswg.org/css2/generate.html#propdef-list-style-position:

   The size or contents of the marker box may affect the height of the principal block box and/or the height of its first line box, and in some cases may cause the creation of a new line box. Note: This interaction may be more precisely defined in a future level of CSS.

So nothing particularly interesting. There are a couple of comments referencing that paragraph in our code.

Now, in terms of what's going on... The test-case is basically an empty list-item acting as a block, with list-style-type: outside.

<!doctype html>
<style>
  .container {
    border: 1px solid black;
    height: 60px;
  }
  .child {
    border: 2px solid orange;
  }
</style>
<div class="container" style="display: list-item">
  <div class="child"></div>
</div>

We have a couple interesting places in our block reflow code. First of all the marker frame is not added to any child list (as opposed to what would happen with list-style-position: inside) when we create the marker.

That generates a frame tree like:

Block(div)(0)@7efc406f9520 parent=7efc406f8ba0 {0,0,35640,3720} vis-overflow=-840,0,36480,3720 scr-overflow=-840,0,36480,3720 [state=0000128040100210] [content=7efc4071c5e0] [sc=7efc406f8af8]<
  line 7efc406f9de8: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8] {60,60,35520,240} <
    Block(div)(1)@7efc406f9c08 parent=7efc406f9520 {60,60,35520,240} [state=0000128000100200] [content=7efc4071c790] [sc=7efc406f9a40]<
    >
  >
  BulletList 0x7efc40923310 <
    Bullet(div)(0)@7efc40923268 parent=7efc406f9520 {-840,60,840,473} [state=0000008000000000] [content=7efc4071c5e0] [sc=7efc406f9780:-moz-list-bullet]
  >
>

Nothing particularly unexpected so far.

Now we have two interesting places, I think. The first one is:

    https://searchfox.org/mozilla-central/rev/84cea84b12145d752e50ddca6be5462c38510e35/layout/generic/nsBlockFrame.cpp#4494

And we can't hit that at all because our kid is a block.

The second one we do hit and pass the condition:

    https://searchfox.org/mozilla-central/rev/84cea84b12145d752e50ddca6be5462c38510e35/layout/generic/nsBlockFrame.cpp#1290

Now whether we do or not can't affect the first line anymore, though.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

I've verified that adding the bullet frame to the in flows calling AddFrames right as we do for the inside bullet fixes the test-case. Obviously it breaks all the other outside bullet test-cases as expected...

I'm still not sure how to make any sense of the other browser's behavior here, both spec-wise and behavior-wise: David, do you know whether the behavior here from other browsers is sane?

See comment 14 for a description of the minimal test-case, or read the previous comments, but the TL;DR is: Other browsers seem to do layout with the outside bullet the same way as we'd do for list-style-position: inside IFF the only in flow contents of that list-item are empty blocks.

Comment 16

[David Baron :dbaron:]

From a typographic perspective, I think the ideal behavior in this case is that a line box ends up being created *inside* the empty block, but I don't think anyone does that.  Our behavior of aligning the bullet with a block rather than a line isn't great, but I think it's probably preferable to creating an empty line box where Chrome creates it.  I think we have some old bugs on improving our behavior.  (Some people who come from an editing perspective have different preferences, but I disagree with them, and note that with this solution it's still possible to get what the editor people want, they just need something nonempty in their box like an  .)

It's not clear to me whether 'list-style-type: none' should suppress the marker box (and its resulting layout effects) or just yield no marker in the marker box.

It's not clear to me that this one site makes it worth changing our behavior to something that's typographically less good; it seems like they're already working around the Chrome behavior somehow.  Were they previously working around the difference with @moz-document?  It seems like it's worth reaching out and suggesting they just stop using list-items...

Comment 17

[David Baron :dbaron:]

(By "list items", I mean things that are 'display: list-item'; it's fine if they want to use <li>s.)

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Sounds good, thanks for the fast reply David!

Over to Tech Evangelism I guess. I've mailed them already.

Comment 19

[Firefox Bug Husbandry Bot]

See bug 1547409. Moving webcompat whiteboard tags to keywords.

Comment 20

[Jeroen Kruis]

Described behaviour is no longer present. Looks like the site has been updated.