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)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: markus.popp, Unassigned)
References
Details
(Keywords: webcompat:site-wait, Whiteboard: [sitewait])
Attachments
(5 files, 1 obsolete file)
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.
Reporter | ||
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
Regression window: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4ad2150e73c0b19f1f6264a4b9ad6927986d43ce&tochange=37e0bd919af057d44c5c1410458c0f00a3653c11
Blocks: 1035091
Flags: needinfo?(emilio)
Reporter | ||
Comment 3•6 years ago
|
||
(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•6 years ago
|
||
Thanks for the report :)
Updated•6 years ago
|
Attachment #8948027 -
Attachment mime type: text/plain → text/html
Comment 5•6 years ago
|
||
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
Updated•6 years ago
|
Attachment #8948031 -
Attachment mime type: text/plain → text/html
Flags: needinfo?(emilio)
Comment 6•6 years ago
|
||
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•6 years ago
|
||
(or difference. In particular, Blink's behavior looks somewhat borked / inconsistent to me off-hand)
Comment 8•6 years ago
|
||
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•6 years ago
|
||
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
Comment 10•6 years ago
|
||
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•6 years ago
|
||
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.
Updated•6 years ago
|
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
Comment 12•6 years ago
|
||
https://searchfox.org/mozilla-central/rev/e06af9c36a73a27864302cd2f829e6200dee8541/layout/generic/nsBlockFrame.cpp#1284 looks suspicious...
Comment 13•6 years ago
|
||
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
Updated•6 years ago
|
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)
Comment 14•6 years ago
|
||
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•6 years ago
|
||
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)
Comment 16•6 years ago
|
||
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...
Flags: needinfo?(dbaron)
Comment 17•6 years ago
|
||
(By "list items", I mean things that are 'display: list-item'; it's fine if they want to use <li>s.)
Comment 18•6 years ago
|
||
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
![]() |
||
Updated•6 years ago
|
Whiteboard: [sitewait]
Updated•6 years ago
|
Priority: -- → P1
Assignee | ||
Updated•5 years ago
|
Product: Tech Evangelism → Web Compatibility
Comment 19•5 years ago
|
||
See bug 1547409. Moving webcompat whiteboard tags to keywords.
Keywords: webcompat:site-wait
Comment 20•2 years ago
|
||
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.
Description
•