[css-pseudo] Move all ::marker style adjustments into StyleAdjuster
LayoutNGListItem was applying some style adjustments to markers, like
'white-space: pre' to preserve trailing spaces in outside markers.
But this was only affecting markers with 'content: normal', and the
changes weren't exposed in getComputedStyle().
This patch moves these adjustments into StyleAdjuster, so that all
markers are affected equally.
BUG=457718
TEST=external/wpt/css/css-pseudo/marker-content-018.html
The test fails in legacy because the 'content' property is not supported
yet in ::marker.
There is a new failure in marker-supported-properties.html because now
'white-space' will compute to 'pre' in outside markers, even if you
specify a different value. But normal markers were already enforcing
'pre' at used value time anyways.
Change-Id: I6d8b8ec7fb26cb04402032924f87b2c3b8fdb9cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985966
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#728176}
diff --git a/third_party/blink/renderer/core/css/resolver/style_adjuster.cc b/third_party/blink/renderer/core/css/resolver/style_adjuster.cc
index eab34ff..303757f 100644
--- a/third_party/blink/renderer/core/css/resolver/style_adjuster.cc
+++ b/third_party/blink/renderer/core/css/resolver/style_adjuster.cc
@@ -50,6 +50,7 @@
#include "third_party/blink/renderer/core/html/html_table_cell_element.h"
#include "third_party/blink/renderer/core/html/media/html_media_element.h"
#include "third_party/blink/renderer/core/html_names.h"
+#include "third_party/blink/renderer/core/layout/layout_list_marker.h"
#include "third_party/blink/renderer/core/layout/layout_object.h"
#include "third_party/blink/renderer/core/layout/layout_replaced.h"
#include "third_party/blink/renderer/core/layout/layout_theme.h"
@@ -208,10 +209,24 @@
(IsA<HTMLLIElement>(parent_element) &&
!parent_style.IsInsideListElement());
- // Outside list markers should generate a block container.
- if (!is_inside) {
+ if (is_inside) {
+ auto margins = LayoutListMarker::InlineMarginsForInside(
+ style, parent_style.GeneratesMarkerImage());
+ style.SetMarginStart(Length::Fixed(margins.first));
+ style.SetMarginEnd(Length::Fixed(margins.second));
+ } else {
+ // Outside list markers should generate a block container.
DCHECK_EQ(style.Display(), EDisplay::kInline);
style.SetDisplay(EDisplay::kInlineBlock);
+
+ // Do not break inside the marker, and honor the trailing spaces.
+ style.SetWhiteSpace(EWhiteSpace::kPre);
+
+ // Compute margins for 'outside' during layout, because it requires the
+ // layout size of the marker.
+ // TODO(kojii): absolute position looks more reasonable, and maybe required
+ // in some cases, but this is currently blocked by crbug.com/734554
+ // style.SetPosition(EPosition::kAbsolute);
}
}
diff --git a/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.cc b/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.cc
index 5eb50dd..e45c6a2 100644
--- a/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.cc
+++ b/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.cc
@@ -152,34 +152,21 @@
is_marker_text_updated_ = true;
return;
}
- scoped_refptr<ComputedStyle> marker_style =
- cached_marker_style ? ComputedStyle::Clone(*cached_marker_style)
- : list_item->StyleForPseudoElement(kPseudoIdMarker);
- DCHECK(marker_style);
if (IsInside()) {
if (marker_ && !marker_->IsLayoutInline())
DestroyMarker();
if (!marker_)
marker_ = LayoutNGInsideListMarker::CreateAnonymous(&GetDocument());
- auto margins =
- LayoutListMarker::InlineMarginsForInside(style, IsMarkerImage());
- marker_style->SetMarginStart(Length::Fixed(margins.first));
- marker_style->SetMarginEnd(Length::Fixed(margins.second));
} else {
if (marker_ && !marker_->IsLayoutBlockFlow())
DestroyMarker();
if (!marker_)
marker_ = LayoutNGListMarker::CreateAnonymous(&GetDocument());
- // Do not break inside the marker, and honor the trailing spaces.
- marker_style->SetWhiteSpace(EWhiteSpace::kPre);
- // Compute margins for 'outside' during layout, because it requires the
- // layout size of the marker.
- // TODO(kojii): absolute position looks more reasonable, and maybe required
- // in some cases, but this is currently blocked by crbug.com/734554
- // marker_style->SetPosition(EPosition::kAbsolute);
- // marker_->SetPositionState(EPosition::kAbsolute);
}
- marker_->SetStyle(std::move(marker_style));
+ if (cached_marker_style)
+ marker_->SetStyle(cached_marker_style);
+ else
+ marker_->SetStyle(list_item->StyleForPseudoElement(kPseudoIdMarker));
UpdateMarkerContentIfNeeded();
diff --git a/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h b/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h
index d6fd7b52..eadcf33 100644
--- a/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h
+++ b/third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h
@@ -26,10 +26,7 @@
static String TextAlternative(const LayoutObject& marker);
LayoutObject* Marker() const { return marker_; }
- bool IsMarkerImage() const {
- return StyleRef().ListStyleImage() &&
- !StyleRef().ListStyleImage()->ErrorOccurred();
- }
+ bool IsMarkerImage() const { return StyleRef().GeneratesMarkerImage(); }
void UpdateMarkerTextIfNeeded() {
if (marker_ && !is_marker_text_updated_ && !IsMarkerImage())
diff --git a/third_party/blink/renderer/core/style/computed_style.h b/third_party/blink/renderer/core/style/computed_style.h
index 08627fe..92fcc53 100644
--- a/third_party/blink/renderer/core/style/computed_style.h
+++ b/third_party/blink/renderer/core/style/computed_style.h
@@ -2573,6 +2573,11 @@
static_cast<unsigned>(RenderSubtreeFlags::kSkipViewportActivation);
}
+ bool GeneratesMarkerImage() const {
+ return Display() == EDisplay::kListItem && ListStyleImage() &&
+ !ListStyleImage()->ErrorOccurred();
+ }
+
private:
EClear Clear() const { return ClearInternal(); }
EFloat Floating() const { return FloatingInternal(); }
diff --git a/third_party/blink/web_tests/FlagExpectations/disable-layout-ng b/third_party/blink/web_tests/FlagExpectations/disable-layout-ng
index 5f1d4ac..3c31098 100644
--- a/third_party/blink/web_tests/FlagExpectations/disable-layout-ng
+++ b/third_party/blink/web_tests/FlagExpectations/disable-layout-ng
@@ -79,6 +79,7 @@
crbug.com/457718 external/wpt/css/css-pseudo/marker-content-015.html [ Failure ]
crbug.com/457718 external/wpt/css/css-pseudo/marker-content-016.html [ Failure ]
crbug.com/457718 external/wpt/css/css-pseudo/marker-content-017.html [ Failure ]
+crbug.com/457718 external/wpt/css/css-pseudo/marker-content-018.html [ Failure ]
crbug.com/1012289 external/wpt/css/css-pseudo/marker-unicode-bidi-default.html [ Failure ]
crbug.com/1012289 external/wpt/css/css-pseudo/marker-unicode-bidi-normal.html [ Failure ]
diff --git a/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-018-ref.html b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-018-ref.html
new file mode 100644
index 0000000..8107d42
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-018-ref.html
@@ -0,0 +1,61 @@
+<!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>
+ol {
+ float: left;
+ width: 100px;
+}
+.inside {
+ list-style-position: inside;
+}
+li:nth-child(1) { list-style-type: "1" }
+li:nth-child(2) { list-style-type: "1 " }
+li:nth-child(3) { list-style-type: "1 " }
+li:nth-child(4) { list-style-type: " 1" }
+li:nth-child(5) { list-style-type: " 1" }
+li:nth-child(6) { list-style-type: " 1 " }
+li:nth-child(7) { list-style-type: "1\9 2" }
+li:nth-child(8) { list-style-type: "1\a 2" }
+</style>
+<ol class="inside">
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+</ol>
+<ol class="inside">
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+</ol>
+<ol class="outside">
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+</ol>
+<ol class="outside">
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+</ol>
diff --git a/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-018.html b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-018.html
new file mode 100644
index 0000000..0a4a73cd
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-018.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<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-018-ref.html">
+<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#marker-pseudo">
+<link rel="help" href="https://drafts.csswg.org/css-text-3/#white-space-rules">
+<meta name="assert" content="Checks that the 'content' property of a ::marker doesn't affect white space.">
+<style>
+ol {
+ float: left;
+ width: 100px;
+}
+.inside {
+ list-style-position: inside;
+}
+li:nth-child(1)::marker { content: "1" }
+li:nth-child(2)::marker { content: "1 " }
+li:nth-child(3)::marker { content: "1 " }
+li:nth-child(4)::marker { content: " 1" }
+li:nth-child(5)::marker { content: " 1" }
+li:nth-child(6)::marker { content: " 1 " }
+li:nth-child(7)::marker { content: "1\9 2" }
+li:nth-child(8)::marker { content: "1\a 2" }
+</style>
+<ol class="inside">
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+</ol>
+<ol class="inside">
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+</ol>
+<ol class="outside">
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+ <li>item</li>
+</ol>
+<ol class="outside">
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+ <li> item</li>
+</ol>
diff --git a/third_party/blink/web_tests/external/wpt/css/css-pseudo/parsing/marker-supported-properties-expected.txt b/third_party/blink/web_tests/external/wpt/css/css-pseudo/parsing/marker-supported-properties-expected.txt
index 880e31a..ae332b1 100644
--- a/third_party/blink/web_tests/external/wpt/css/css-pseudo/parsing/marker-supported-properties-expected.txt
+++ b/third_party/blink/web_tests/external/wpt/css/css-pseudo/parsing/marker-supported-properties-expected.txt
@@ -15,7 +15,7 @@
PASS Property font-variant-numeric value 'slashed-zero' in ::marker
FAIL Property font-variant-position value 'sub' in ::marker assert_true: font-variant-position doesn't seem to be supported in the computed style expected true got false
PASS Property font-weight value '900' in ::marker
-PASS Property white-space value 'nowrap' in ::marker
+FAIL Property white-space value 'nowrap' in ::marker assert_equals: expected "nowrap" but got "pre"
PASS Property color value 'rgb(0, 100, 200)' in ::marker
PASS Property text-combine-upright value 'all' in ::marker
PASS Property unicode-bidi value 'plaintext' in ::marker
