Closed Bug 1435385 Opened 6 years ago Closed 2 years ago

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)

Categories

(Web Compatibility :: Desktop, defect, P1)

Firefox 59
defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: markus.popp, Unassigned)

References

Details

(Keywords: webcompat:site-wait, Whiteboard: [sitewait])

Attachments

(5 files, 1 obsolete file)

Attached image 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.
Attached image Rendering in Firefox 59
(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.
Attached file testcase (obsolete) —
Thanks for the report :)
Attachment #8948027 - Attachment mime type: text/plain → text/html
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...
Attachment #8948027 - Attachment is obsolete: true
Attachment #8948031 - Attachment mime type: text/plain → text/html
Flags: needinfo?(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.
(or difference. In particular, Blink's behavior looks somewhat borked / inconsistent to me off-hand)
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 on attachment 8948037 [details]
Even more simplified test-case.

I should learn to just attach the file instead of copy-pasting it...
Attachment #8948037 - Attachment mime type: text/plain → text/html
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
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.
Attachment #8948050 - Attachment description: testcase with empty div (orange line) inside of container div of various flavors → testcase with empty vs. nonempty div (orange line or box) inside of container div of various flavors
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.
Component: Layout: View Rendering → Layout
Summary: Rendering difference between Firefox 58 and Firefox 59 → Firefox disagrees with Chrome/Edge on vertical placement of empty "display: list-item" elements
Summary: Firefox disagrees with Chrome/Edge on vertical placement of empty "display: list-item" elements → 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)
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.
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.
Flags: needinfo?(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 &nbsp;.)

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...
Flags: needinfo?(dbaron)
(By "list items", I mean things that are 'display: list-item'; it's fine if they want to use <li>s.)
Sounds good, thanks for the fast reply David!

Over to Tech Evangelism I guess. I've mailed them already.
Component: Layout → Desktop
Product: Core → Tech Evangelism
Version: 59 Branch → Firefox 59
Whiteboard: [sitewait]
Priority: -- → P1
Product: Tech Evangelism → Web Compatibility

See bug 1547409. Moving webcompat whiteboard tags to keywords.

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

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.