Skip to content

GitLab

T
tangled
Commit fd8e4a0d authored by Oriol Brufau's avatar Oriol Brufau Committed by Commit Bot
Browse files
Options

[css-lists] Avoid newline when changing list marker position in legacy 

Dynamic changes to 'list-style-position' in legacy layout are so broken,
especially with nested list items. In lots of cases, some markers end up
nested inside the wrong list item, or appear in the wrong position, or
an empty line is inserted somewhere.

This patch doesn't fix all the cases, which would require a big
refactorization. It only fixes the case that I need for bug 457718.

The case is:
- Initially,
  <div><div style="list-style-position: inside"><div></div></div></div>
- After being laid out, the middle <div> is set to 'outside':
  <div><div><div></div></div></div>
- After reflowing, the inner <div> is set to 'inside':
  <div><div><div style="list-style-position: inside"></div></div></div>

This used to place the 2 outside markers in a 1st line and the inside
one in a 2nd line, creating an empty line between them.
This patch fixes it.

BUG=1051114

TEST=web_tests/external/wpt/css/css-lists/change-list-style-position-002.html

The patch also fixes some typos in change-list-style-position-001.html

Change-Id: Ib42433ec43eca1925d0a47f4caaabbce7951360c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049854Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#740617}
parent 297307a0
Show whitespace changes
Inline Side-by-side
Showing with 45 additions and 8 deletions
third_party/blink/renderer/core/layout/layout_list_item.cc
View file @ fd8e4a0d
...@@ -117,8 +117,7 @@ bool LayoutListItem::IsEmpty() const { ...@@ -117,8 +117,7 @@ bool LayoutListItem::IsEmpty() const {
namespace { namespace {
LayoutObject* GetParentOfFirstLineBox(LayoutBlockFlow* curr, LayoutObject* GetParentOfFirstLineBox(LayoutBlockFlow* curr) {
LayoutObject* marker) {
LayoutObject* first_child = curr->FirstChild(); LayoutObject* first_child = curr->FirstChild();
if (!first_child) if (!first_child)
return nullptr; return nullptr;
...@@ -126,7 +125,7 @@ LayoutObject* GetParentOfFirstLineBox(LayoutBlockFlow* curr, ...@@ -126,7 +125,7 @@ LayoutObject* GetParentOfFirstLineBox(LayoutBlockFlow* curr,
bool in_quirks_mode = curr->GetDocument().InQuirksMode(); bool in_quirks_mode = curr->GetDocument().InQuirksMode();
for (LayoutObject* curr_child = first_child; curr_child; for (LayoutObject* curr_child = first_child; curr_child;
curr_child = curr_child->NextSibling()) { curr_child = curr_child->NextSibling()) {
if (curr_child == marker) if (curr_child->IsOutsideListMarker())
continue; continue;
if (curr_child->IsInline() && if (curr_child->IsInline() &&
...@@ -150,7 +149,7 @@ LayoutObject* GetParentOfFirstLineBox(LayoutBlockFlow* curr, ...@@ -150,7 +149,7 @@ LayoutObject* GetParentOfFirstLineBox(LayoutBlockFlow* curr,
IsA<HTMLOListElement>(*curr_child->GetNode()))) IsA<HTMLOListElement>(*curr_child->GetNode())))
break; break;
LayoutObject* line_box = GetParentOfFirstLineBox(child_block_flow, marker); LayoutObject* line_box = GetParentOfFirstLineBox(child_block_flow);
if (line_box) if (line_box)
return line_box; return line_box;
} }
...@@ -251,7 +250,7 @@ bool LayoutListItem::UpdateMarkerLocation() { ...@@ -251,7 +250,7 @@ bool LayoutListItem::UpdateMarkerLocation() {
LayoutObject* line_box_parent = nullptr; LayoutObject* line_box_parent = nullptr;
if (!marker_->IsInside()) if (!marker_->IsInside())
line_box_parent = GetParentOfFirstLineBox(this, marker_); line_box_parent = GetParentOfFirstLineBox(this);
if (line_box_parent && (line_box_parent->HasOverflowClip() || if (line_box_parent && (line_box_parent->HasOverflowClip() ||
!line_box_parent->IsLayoutBlockFlow() || !line_box_parent->IsLayoutBlockFlow() ||
(line_box_parent->IsBox() && (line_box_parent->IsBox() &&
...@@ -332,7 +331,7 @@ void LayoutListItem::AlignMarkerInBlockDirection() { ...@@ -332,7 +331,7 @@ void LayoutListItem::AlignMarkerInBlockDirection() {
// layout pass. So if there's no line box in line_box_parent make sure it // layout pass. So if there's no line box in line_box_parent make sure it
// back to its original position. // back to its original position.
bool back_to_original_baseline = false; bool back_to_original_baseline = false;
LayoutObject* line_box_parent = GetParentOfFirstLineBox(this, marker_); LayoutObject* line_box_parent = GetParentOfFirstLineBox(this);
LayoutBox* line_box_parent_block = nullptr; LayoutBox* line_box_parent_block = nullptr;
if (!line_box_parent || !line_box_parent->IsBox()) { if (!line_box_parent || !line_box_parent->IsBox()) {
back_to_original_baseline = true; back_to_original_baseline = true;
......
third_party/blink/web_tests/external/wpt/css/css-lists/change-list-style-position-001-ref.html
View file @ fd8e4a0d
<!DOCTYPE html> <!DOCTYPE html>
<meta charset="utf-8"> <meta charset="utf-8">
<title>CSS Lists: Add inline child after marker</title> <title>CSS Lists: test the change of list-style-position</title>
<p>The test passes if you see the list marker followed by the text "inline" and "axxx" in a line below.</p> <p>The test passes if you see the list marker followed by the text "inline" and "axxx" in a line below.</p>
......
third_party/blink/web_tests/external/wpt/css/css-lists/change-list-style-position-001.html
View file @ fd8e4a0d
<!DOCTYPE html> <!DOCTYPE html>
<meta charset="utf-8"> <meta charset="utf-8">
<title>CSS Lists: test the change of list-style-type</title> <title>CSS Lists: test the change of list-style-position</title>
<link rel=help href="https://www.w3.org/TR/CSS22/generate.html#lists"> <link rel=help href="https://www.w3.org/TR/CSS22/generate.html#lists">
<link rel=match href="change-list-style-position-001-ref.html"> <link rel=match href="change-list-style-position-001-ref.html">
......
third_party/blink/web_tests/external/wpt/css/css-lists/change-list-style-position-002-ref.html 0 → 100644
View file @ fd8e4a0d
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Lists: test the change of list-style-position</title>
<style>
div {
display: list-item;
margin-left: 40px;
border: 5px solid;
}
div > div { list-style-type: decimal }
div > div > div { list-style-type: lower-roman }
.inside { list-style-position: inside }
</style>
<div><div><div class="inside">text</div></div></div>
third_party/blink/web_tests/external/wpt/css/css-lists/change-list-style-position-002.html 0 → 100644
View file @ fd8e4a0d
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Lists: test the change of list-style-position</title>
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
<link rel="match" href="change-list-style-position-002-ref.html">
<link rel="help" href="https://www.w3.org/TR/CSS22/generate.html#lists">
<style>
div {
display: list-item;
margin-left: 40px;
border: 5px solid;
}
div > div { list-style-type: decimal }
div > div > div { list-style-type: lower-roman }
.inside { list-style-position: inside }
</style>
<div><div id="bar"><div id="baz">text</div></div></div>
<script>
document.getElementById("bar").className = "inside";
document.body.offsetHeight;
document.getElementById("bar").className = "";
document.body.offsetHeight;
document.getElementById("baz").className = "inside";
</script>
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