Commit d1a95a60 authored by Oriol Brufau's avatar Oriol Brufau Committed by Commit Bot

[css-pseudo] Avoid empty line in legacy list-item when inserting node

Consider <ol><li><span><div>foo</div></span></li></ol>

Chromium shows "foo" just next to the list marker. However, in legacy
layout, if the <div> was inserted dynamically, then a newline was added
between them.

Also consider
<ul><li><span></span><div style="overflow:hidden">bar</div></li></ul>

Chromium shows "bar" just next to the list marker. However, in legacy
layout, if the <span> was inserted dynamically, then a newline was added
between them.

This patch fixes these inconsistent behaviors.

BUG=1049633

TEST=external/wpt/css/css-lists/add-inline-child-after-marker-002.html
TEST=external/wpt/css/css-pseudo/marker-content-022.html

Change-Id: I11a88a6b375436c72a26c5cd7bcedbb7a197b39d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041481Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#739423}
parent 192e0222
......@@ -189,17 +189,21 @@ bool LayoutListItem::PrepareForBlockDirectionAlign(
LayoutObject* marker_parent = marker_->Parent();
// Deal with the situation of layout tree changed.
if (marker_parent && marker_parent->IsAnonymous()) {
bool marker_parent_has_lines =
line_box_parent && line_box_parent->IsDescendantOf(marker_parent);
// When list-position-style change from outside to inside, we need to
// restore LogicalHeight to auto. So add IsInside().
if (marker_->IsInside() || marker_->NextSibling()) {
if (marker_->IsInside() || marker_parent_has_lines) {
// Set marker_container's LogicalHeight to auto.
if (marker_parent->StyleRef().LogicalHeight().IsZero())
ForceLogicalHeight(*marker_parent, Length());
// If marker_parent isn't the ancestor of line_box_parent, marker might
// generate a new empty line. We need to remove marker here.E.g:
// <li><span><div>text<div><span></li>
if (line_box_parent && !line_box_parent->IsDescendantOf(marker_parent)) {
// If marker_parent_has_lines and the marker is outside, we need to move
// the marker into another parent with 'height: 0' to avoid generating a
// new empty line in cases like <li><span><div>text<div><span></li>
// If the marker is inside and there are inline contents, we want them to
// share the same block container to avoid a line break between them.
if (marker_->IsInside() != marker_parent_has_lines) {
marker_->Remove();
marker_parent = nullptr;
}
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Lists: Add inline child after marker</title>
<p>The test passes if you see the list marker followed by the text "axxx" in the same line.</p>
<ul>
<li>
<span> </span>
<div style="overflow:hidden;">
<span>a</span>xxx
</div>
</li>
</ul>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Lists: Add inline child after marker</title>
<link rel=help href="https://www.w3.org/TR/CSS22/generate.html#lists">
<link rel=match href="add-inline-child-after-marker-002-ref.html">
<!-- https://bugs.chromium.org/p/chromium/issues/detail?id=1049633 -->
<p>The test passes if you see the list marker followed by the text "axxx" in the same line.</p>
<ul>
<li id="liTarget">
<div id="divTarget" style="overflow:hidden;">
<span>a</span>xxx
</div>
</li>
</ul>
<script>
document.body.offsetHeight;
var new_span=document.createElement("span");
var text_node=document.createTextNode(" ");
new_span.appendChild(text_node);
var div_target=document.getElementById("divTarget");
var li_target=document.getElementById("liTarget");
li_target.insertBefore(new_span,div_target);
</script>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Lists: Add inline child after marker</title>
<p>The test passes if you see the list marker followed by the text "inline" and "axxx" in a line below.</p>
<ul>
<li style="list-style-position:inside;">
<span>inline</span>
<div style="overflow:hidden;">
<span>a</span>xxx
</div>
</li>
</ul>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Lists: test the change of list-style-type</title>
<link rel=help href="https://www.w3.org/TR/CSS22/generate.html#lists">
<link rel=match href="change-list-style-position-001-ref.html">
<p>The test passes if you see the list marker followed by the text "inline" and "axxx" in a line below.</p>
<ul>
<li>
<span>inline</span>
<div style="overflow:hidden;">
<span>a</span>xxx
</div>
</li>
</ul>
<script>
document.body.offsetHeight;
document.querySelector("li").style.listStylePosition = "inside";
</script>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Reference: ::marker pseudo elements styled with 'content' property</title>
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
<style>
.symbol {
list-style-type: disc;
}
.decimal {
list-style-type: decimal;
}
.string {
list-style-type: "string";
}
.content::marker {
content: "content";
}
</style>
<ol>
<li class="symbol"><span><div>foo</div></span></li>
<li class="decimal"><span><div>foo</div></span></li>
<li class="string"><span><div>foo</div></span></li>
<li class="content"><span><div>foo</div></span></li>
</ol>
<!DOCTYPE html>
<html class="reftest-wait">
<meta charset="utf-8">
<title>CSS Test: ::marker pseudo elements styled with 'content' property</title>
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
<link rel="match" href="marker-content-022-ref.html">
<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#marker-pseudo">
<link rel="help" href="https://drafts.csswg.org/css-lists/#list-style-position-outside">
<meta name="assert" content="When a list item with an outside marker only has an inline child
with a block grandchild, some browsers insert a newline between the marker and the block,
and some don't. It's not clear what should happen, but this test checks that the behavior
is consistent whether the block is inserted dynamically or was there from the beginning.">
<style>
.symbol {
list-style-type: disc;
}
.decimal {
list-style-type: decimal;
}
.string {
list-style-type: "string";
}
.content::marker {
content: "content";
}
</style>
<ol>
<li class="symbol"><span></span></li>
<li class="decimal"><span></span></li>
<li class="string"><span></span></li>
<li class="content"><span></span></li>
</ol>
<script src="/common/reftest-wait.js"></script>
<script>
// Use a "load" event listener and requestAnimationFrame to ensure that
// the markers will have been laid out.
addEventListener("load", () => requestAnimationFrame(() => {
const div = document.createElement("div");
div.appendChild(document.createTextNode("foo"));
for (let span of document.querySelectorAll("li > span")) {
span.appendChild(div.cloneNode(true));
}
takeScreenshot();
}), {once: true});
</script>
</html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment