List-Item Bullet displayed at wrong location on first layout pass if first box is an inline containing a block
Reported by
francois...@outlook.com,
Apr 25 2017
|
||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3075.0 Safari/537.36 Steps to reproduce the problem: https://wptest.center/#/4apjp4 What is the expected behavior? Both elements should render the same What went wrong? The second one is missing an empty line containing the leading space Did this work before? N/A Does this work in other browsers? Yes Chrome version: 60.0.3075.0 Channel: n/a OS Version: 10.0 Flash Version:
,
May 1 2017
,
May 2 2017
This has a bit of history, and dates from https://bugs.webkit.org/show_bug.cgi?id=14395. The test added there is at: https://jsfiddle.net/q3v4ztt1/2/ Each of Edge/FF/Chrome renders it a bit differently. As you would expect, it looks like we changed our behaviour to match IE/FF at the time. Also, I don't think the right behaviour is specified anywhere. Anyone got any suggestions?
,
May 18 2017
I have no strong opinion myself, but I think the two cases I provided should do the same thing. The easiest seems to always include the empty line. I think all options will break some site anyway, so we should aim for consistency.
,
May 19 2017
,
May 22 2017
Apart from what we should do, re-layout causing different result looks like an invalidation bug to me.
,
May 23 2017
kojii@ I don't think there's unstable layout here - just different layout depending on whether there's leading space or not. I think I'll take Francois' suggestion and make our treatment consistent like Edge.
,
May 24 2017
The summary says "on first layout pass". I'm looking at the original test case: https://wptest.center/#/4apjp4 and clicking "Force relayout" makes both look the same, so I thought this is an invalidation problem. I don't understand tests in #3...none of them have empty lines, and I can't see toggling display:none changes these tests, are they testing the same condition? Can you explain why #1 changes layout by toggling while yours don't? I don't have strong opinion either, but I'd like to add that in LayoutNG, block in inline is causing several issues and we recognized this is unspecified.
,
May 28 2017
kojii@ - You're right, #3 is related but not the specific bug at issue here. Though fixing #3 makes it go away:
<!DOCTYPE html>
<style>
li { border: 3px solid red; margin: 3px; }
img { display: block; }
</style>
<ul>
<li>
<a href="#"><img width=32 height=32 /></a>
Some other text
</li>
</ul>
<script>
document.body.offsetTop;
var img = document.querySelector('a img');
img.style.display = 'inline';
img.offsetWidth;
img.style.display = 'block';
</script>
Here's the layout tree when you run the above:
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x110
LayoutBlockFlow {HTML} at (0,0) size 800x110
LayoutBlockFlow {BODY} at (8,16) size 784x78
LayoutBlockFlow {UL} at (0,0) size 784x78
LayoutListItem {LI} at (43,0) size 738x78 [border: (3px solid #FF0000)]
LayoutBlockFlow (anonymous) at (3,3) size 732x20
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
LayoutListMarker (anonymous) at (-21,0) size 7x19: bullet
LayoutInline {A} at (0,0) size 0x19 [color=#0000EE]
LayoutBlockFlow (anonymous) at (3,23) size 732x32 [color=#0000EE]
LayoutImage {IMG} at (0,0) size 32x32
LayoutBlockFlow (anonymous) at (3,55) size 732x20
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
LayoutText {#text} at (0,0) size 99x19
text run at (0,0) width 99: "Some other text"
Here's what you get if you remove the <script> part:
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x90
LayoutBlockFlow {HTML} at (0,0) size 800x90
LayoutBlockFlow {BODY} at (8,16) size 784x58
LayoutBlockFlow {UL} at (0,0) size 784x58
LayoutListItem {LI} at (43,0) size 738x58 [border: (3px solid #FF0000)]
LayoutBlockFlow (anonymous) at (3,3) size 732x0
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
LayoutBlockFlow (anonymous) at (3,3) size 732x32 [color=#0000EE]
LayoutImage {IMG} at (0,0) size 32x32
LayoutBlockFlow (anonymous) at (3,35) size 732x20
LayoutListMarker (anonymous) at (-21,0) size 7x19: bullet
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
LayoutText {#text} at (0,0) size 99x19
text run at (0,0) width 99: "Some other text"
So this bug is about how we clean up the unnecessary extra inline continuations in:
LayoutBlockFlow (anonymous) at (3,3) size 732x20
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
LayoutListMarker (anonymous) at (-21,0) size 7x19: bullet <---- this guy should be in the third anon block
LayoutInline {A} at (0,0) size 0x19 [color=#0000EE] <--- this guy is superfluous
LayoutBlockFlow (anonymous) at (3,23) size 732x32 [color=#0000EE]
LayoutImage {IMG} at (0,0) size 32x32
LayoutBlockFlow (anonymous) at (3,55) size 732x20
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
to get:
LayoutBlockFlow (anonymous) at (3,3) size 732x0
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
LayoutBlockFlow (anonymous) at (3,3) size 732x32 [color=#0000EE]
LayoutImage {IMG} at (0,0) size 32x32
LayoutBlockFlow (anonymous) at (3,35) size 732x20
LayoutListMarker (anonymous) at (-21,0) size 7x19: bullet
LayoutInline {A} at (0,0) size 0x0 [color=#0000EE]
LayoutText {#text} at (0,0) size 99x19
,
May 29 2017
#3 - > Each of Edge/FF/Chrome renders it a bit differently. As you would expect, it looks like we changed our behaviour to match IE/FF at the time. Also, I don't think the right behaviour is specified anywhere. It would be great if you discussed this in the CSSWG or filed an issue for the differences and unclear or underspecified features.
,
May 30 2018
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 30 2018
,
Jun 1 2018
#9 is very clear. But after we implemented the zero-height container of marker, the case in #9 would hit the zero-height container in the latest version. After creating the container, it'll be align the block position adjust the first line box. Here is what going on in this case: Step 1. <img> is block, it'll find the first part of <a> as baseline anchor. This <a> dose not have inline box. So the block position won't be adjusted. Step 2. <img> is inline. The layout tree's changed. It could find a line box to adjust its position. So the y-position is changed. Step 3. <img> is changed back to block. As the process in step 1, it won't adjust the position in block direction. So the position will remain as step 2. At step 3 we need to restore the position to step 1. Then the appearance will be similar to Edge, but without the leading empty line.
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28b6d51b55308bc3d8562414859640161d0d3e93 commit 28b6d51b55308bc3d8562414859640161d0d3e93 Author: cathiechen <cathiechen@tencent.com> Date: Mon Jun 04 07:39:44 2018 Fixed marker position error when layout tree changed and then restored While adjust marker's position in block direction, it quit adjusting if it couldn't find a line box. The position of marker is wrong if it changed from without line-box to with line-box, and restored to without line-box. Change the basline anchor to ContainingBlock of marker if couldn't find line box. To make sure the position won't effected by layout tree change. Bug: 715288 Change-Id: Ice59e144cc1c876ba3b5d6ab542052ba0d47d8e8 Reviewed-on: https://chromium-review.googlesource.com/1082136 Commit-Queue: cathie chen <cathiechen@tencent.com> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#564009} [add] https://crrev.com/28b6d51b55308bc3d8562414859640161d0d3e93/third_party/WebKit/LayoutTests/fast/lists/list-with-image-display-changed-expected.html [add] https://crrev.com/28b6d51b55308bc3d8562414859640161d0d3e93/third_party/WebKit/LayoutTests/fast/lists/list-with-image-display-changed.html [modify] https://crrev.com/28b6d51b55308bc3d8562414859640161d0d3e93/third_party/blink/renderer/core/layout/layout_list_item.cc
,
Jun 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e5260130213fa75d6d4ad40051f93c0c6c022b9b commit e5260130213fa75d6d4ad40051f93c0c6c022b9b Author: cathiechen <cathiechen@tencent.com> Date: Fri Jun 08 12:13:07 2018 [LayoutNG] Get the missing ListMarkers back ListMarker won't be shown under these two situation: 1. We update marker text in WillCollectInlines(), it won't be called when ListItem isn't ChildrenInline. Call WillCollectInlines() manually if ListItem only has block children. 2. If marker isn't the child of LI, both LI and marker_parent won't paint marker. So make marker as a direct child of LI. Bug: 715288 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng Change-Id: I994fc9980c73cf00f983142d86b4ba49687d72f6 Reviewed-on: https://chromium-review.googlesource.com/1088333 Reviewed-by: Koji Ishii <kojii@chromium.org> Commit-Queue: cathie chen <cathiechen@tencent.com> Cr-Commit-Position: refs/heads/master@{#565613} [modify] https://crrev.com/e5260130213fa75d6d4ad40051f93c0c6c022b9b/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [modify] https://crrev.com/e5260130213fa75d6d4ad40051f93c0c6c022b9b/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/css2.1/20110323/list-style-position-005-expected.txt [modify] https://crrev.com/e5260130213fa75d6d4ad40051f93c0c6c022b9b/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.cc [modify] https://crrev.com/e5260130213fa75d6d4ad40051f93c0c6c022b9b/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc
,
Jun 8 2018
The position of marker won't change after click the button both in legacy and NG Layout. \o/ However, the appearances are different: Legacy layout: Position the marker at the top-left corner.(See: the attached file) LayoutNG: Position it against the text.(Attach the file latter) Should make them consistent next step. Francois@ Have this filed an issue in W3C? If not, maybe we should do it.:)
,
Jun 15 2018
Filed an issue in w3c:) https://github.com/w3c/csswg-drafts/issues/2787 |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by nyerramilli@google.com
, Apr 28 2017