[css-pseudo] Support 'content: none' for LayoutNG ::marker
Unlike ::before and ::after, in ::marker 'content: normal' and 'none'
should behave differently: the former should display it normally, the
latter should prevent it from generating a box.
Before this patch, 'normal' and 'none' were both stored as nullptr, so
they were indistinguishable. This patch makes 'none' be stored as a
NoneContentData instead.
However, 'none' keeps behaving as 'normal' for anything different than
::marker. In getComputedStyle, 'none' keeps resolving to 'normal' for
anything different than ::marker, ::before and ::after, and 'normal'
keeps resolving to 'none' for ::before and ::after.
Inheritance is not affected either since the 'content' property can't
be inherited in Chromium.
BUG=457718
TEST=external/wpt/css/css-pseudo/marker-computed-content.html
TEST=external/wpt/css/css-pseudo/marker-computed-size.html
TEST=external/wpt/css/css-pseudo/marker-content-019.html
marker-content-019.html fails in legacy.
Change-Id: Ib71ef18dfb7f7c98e76888bffb408a17fa458235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2002517
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#732549}
diff --git a/third_party/blink/renderer/core/css/properties/computed_style_utils.cc b/third_party/blink/renderer/core/css/properties/computed_style_utils.cc
index 131abc6..68eb20e 100644
--- a/third_party/blink/renderer/core/css/properties/computed_style_utils.cc
+++ b/third_party/blink/renderer/core/css/properties/computed_style_utils.cc
@@ -1844,6 +1844,11 @@
CSSValue* ComputedStyleUtils::ValueForContentData(const ComputedStyle& style,
bool allow_visited_style) {
+ if (style.ContentPreventsBoxGeneration())
+ return CSSIdentifierValue::Create(CSSValueID::kNone);
+ if (style.ContentBehavesAsNormal())
+ return CSSIdentifierValue::Create(CSSValueID::kNormal);
+
CSSValueList* outer_list = CSSValueList::CreateSlashSeparated();
CSSValueList* list = CSSValueList::CreateSpaceSeparated();
@@ -1888,12 +1893,7 @@
NOTREACHED();
}
}
- if (!list->length()) {
- PseudoId pseudoId = style.StyleType();
- if (pseudoId == kPseudoIdBefore || pseudoId == kPseudoIdAfter)
- return CSSIdentifierValue::Create(CSSValueID::kNone);
- return CSSIdentifierValue::Create(CSSValueID::kNormal);
- }
+ DCHECK(list->length());
outer_list->Append(*list);
if (alt_text)
diff --git a/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc b/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
index efc1972..99509a5 100644
--- a/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
+++ b/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
@@ -1968,7 +1968,10 @@
if (auto* identifier_value = DynamicTo<CSSIdentifierValue>(value)) {
DCHECK(identifier_value->GetValueID() == CSSValueID::kNormal ||
identifier_value->GetValueID() == CSSValueID::kNone);
- state.Style()->SetContent(nullptr);
+ if (identifier_value->GetValueID() == CSSValueID::kNone)
+ state.Style()->SetContent(MakeGarbageCollected<NoneContentData>());
+ else
+ state.Style()->SetContent(nullptr);
return;
}
const CSSValueList& outer_list = To<CSSValueList>(value);
diff --git a/third_party/blink/renderer/core/dom/pseudo_element.cc b/third_party/blink/renderer/core/dom/pseudo_element.cc
index 8dcd7c6..43bb6f3d 100644
--- a/third_party/blink/renderer/core/dom/pseudo_element.cc
+++ b/third_party/blink/renderer/core/dom/pseudo_element.cc
@@ -184,7 +184,7 @@
ToLayoutNGListItem(layout_object->Parent())
->UpdateMarkerContentIfNeeded();
}
- if (!style.GetContentData())
+ if (style.ContentBehavesAsNormal())
return;
break;
}
@@ -195,7 +195,8 @@
return;
}
- DCHECK(style.GetContentData());
+ DCHECK(!style.ContentBehavesAsNormal());
+ DCHECK(!style.ContentPreventsBoxGeneration());
for (const ContentData* content = style.GetContentData(); content;
content = content->Next()) {
LegacyLayout legacy = context.force_legacy_layout ? LegacyLayout::kForce
@@ -246,10 +247,10 @@
return true;
case kPseudoIdBefore:
case kPseudoIdAfter:
- return pseudo_style->GetContentData();
+ return !pseudo_style->ContentPreventsBoxGeneration();
case kPseudoIdMarker: {
- if (pseudo_style->GetContentData())
- return true;
+ if (!pseudo_style->ContentBehavesAsNormal())
+ return !pseudo_style->ContentPreventsBoxGeneration();
const ComputedStyle* parent_style =
originating_element->GetComputedStyle();
return parent_style &&
diff --git a/third_party/blink/renderer/core/html/html_br_element.cc b/third_party/blink/renderer/core/html/html_br_element.cc
index d81dd2d7..53c7f60 100644
--- a/third_party/blink/renderer/core/html/html_br_element.cc
+++ b/third_party/blink/renderer/core/html/html_br_element.cc
@@ -62,9 +62,9 @@
LayoutObject* HTMLBRElement::CreateLayoutObject(const ComputedStyle& style,
LegacyLayout legacy) {
- if (style.HasContent())
- return LayoutObject::CreateObject(this, style, legacy);
- return new LayoutBR(this);
+ if (style.ContentBehavesAsNormal())
+ return new LayoutBR(this);
+ return LayoutObject::CreateObject(this, style, legacy);
}
} // namespace blink
diff --git a/third_party/blink/renderer/core/html/html_frame_set_element.cc b/third_party/blink/renderer/core/html/html_frame_set_element.cc
index 9d4cb87f..a851271 100644
--- a/third_party/blink/renderer/core/html/html_frame_set_element.cc
+++ b/third_party/blink/renderer/core/html/html_frame_set_element.cc
@@ -221,9 +221,9 @@
LayoutObject* HTMLFrameSetElement::CreateLayoutObject(
const ComputedStyle& style,
LegacyLayout legacy) {
- if (style.HasContent())
- return LayoutObject::CreateObject(this, style, legacy);
- return new LayoutFrameSet(this);
+ if (style.ContentBehavesAsNormal())
+ return new LayoutFrameSet(this);
+ return LayoutObject::CreateObject(this, style, legacy);
}
void HTMLFrameSetElement::AttachLayoutTree(AttachContext& context) {
diff --git a/third_party/blink/renderer/core/layout/layout_list_marker.cc b/third_party/blink/renderer/core/layout/layout_list_marker.cc
index 7ca78af..edb53c65 100644
--- a/third_party/blink/renderer/core/layout/layout_list_marker.cc
+++ b/third_party/blink/renderer/core/layout/layout_list_marker.cc
@@ -312,7 +312,7 @@
std::pair<LayoutUnit, LayoutUnit> LayoutListMarker::InlineMarginsForInside(
const ComputedStyle& style,
bool is_image) {
- if (style.GetContentData())
+ if (!style.ContentBehavesAsNormal())
return {};
if (is_image)
return {LayoutUnit(), LayoutUnit(kCMarkerPaddingPx)};
@@ -332,7 +332,7 @@
LayoutUnit marker_inline_size) {
LayoutUnit margin_start;
LayoutUnit margin_end;
- if (style.GetContentData()) {
+ if (!style.ContentBehavesAsNormal()) {
margin_start = -marker_inline_size;
} else if (is_image) {
margin_start = -marker_inline_size - kCMarkerPaddingPx;
diff --git a/third_party/blink/renderer/core/layout/layout_object.cc b/third_party/blink/renderer/core/layout/layout_object.cc
index 622955da..80866b0 100644
--- a/third_party/blink/renderer/core/layout/layout_object.cc
+++ b/third_party/blink/renderer/core/layout/layout_object.cc
@@ -2532,7 +2532,7 @@
// Don't propagate style from markers with 'content: normal' because it's not
// needed and it would be slow.
- if (pseudo_id == kPseudoIdMarker && !StyleRef().GetContentData())
+ if (pseudo_id == kPseudoIdMarker && StyleRef().ContentBehavesAsNormal())
return;
// Propagate style from pseudo elements to generated content. We skip children
diff --git a/third_party/blink/renderer/core/layout/ng/list/layout_ng_inside_list_marker.h b/third_party/blink/renderer/core/layout/ng/list/layout_ng_inside_list_marker.h
index 826079b..43f353d 100644
--- a/third_party/blink/renderer/core/layout/ng/list/layout_ng_inside_list_marker.h
+++ b/third_party/blink/renderer/core/layout/ng/list/layout_ng_inside_list_marker.h
@@ -20,7 +20,7 @@
#if DCHECK_IS_ON()
void AddChild(LayoutObject* new_child, LayoutObject* before_child) override {
// List markers with 'content: normal' should have at most one child.
- DCHECK(StyleRef().GetContentData() || !FirstChild());
+ DCHECK(!StyleRef().ContentBehavesAsNormal() || !FirstChild());
LayoutInline::AddChild(new_child, before_child);
}
#endif
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 db7b5f4b..baa2e08 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
@@ -274,7 +274,7 @@
LayoutObject* child = marker->SlowFirstChild();
// There should be at most one child.
DCHECK(!child || !child->SlowFirstChild());
- if (marker->StyleRef().GetContentData()) {
+ if (!marker->StyleRef().ContentBehavesAsNormal()) {
marker_type_ = kStatic;
is_marker_text_updated_ = true;
} else if (IsMarkerImage()) {
diff --git a/third_party/blink/renderer/core/style/computed_style.h b/third_party/blink/renderer/core/style/computed_style.h
index 6b0eecd..dbfaa89 100644
--- a/third_party/blink/renderer/core/style/computed_style.h
+++ b/third_party/blink/renderer/core/style/computed_style.h
@@ -2137,7 +2137,25 @@
bool HasIsolation() const { return Isolation() != EIsolation::kAuto; }
// Content utility functions.
- bool HasContent() const { return GetContentData(); }
+ bool ContentBehavesAsNormal() const {
+ switch (StyleType()) {
+ case kPseudoIdMarker:
+ return !GetContentData();
+ default:
+ return !GetContentData() || GetContentData()->IsNone();
+ }
+ }
+ bool ContentPreventsBoxGeneration() const {
+ switch (StyleType()) {
+ case kPseudoIdBefore:
+ case kPseudoIdAfter:
+ return ContentBehavesAsNormal();
+ case kPseudoIdMarker:
+ return GetContentData() && GetContentData()->IsNone();
+ default:
+ return false;
+ }
+ }
// Cursor utility functions.
CursorList* Cursors() const { return CursorDataInternal().Get(); }
diff --git a/third_party/blink/renderer/core/style/content_data.cc b/third_party/blink/renderer/core/style/content_data.cc
index 549b833cc..41679038 100644
--- a/third_party/blink/renderer/core/style/content_data.cc
+++ b/third_party/blink/renderer/core/style/content_data.cc
@@ -110,4 +110,12 @@
return layout_object;
}
+LayoutObject* NoneContentData::CreateLayoutObject(
+ PseudoElement& pseudo,
+ const ComputedStyle& pseudo_style,
+ LegacyLayout) const {
+ NOTREACHED();
+ return nullptr;
+}
+
} // namespace blink
diff --git a/third_party/blink/renderer/core/style/content_data.h b/third_party/blink/renderer/core/style/content_data.h
index 60ec585..ea892c0 100644
--- a/third_party/blink/renderer/core/style/content_data.h
+++ b/third_party/blink/renderer/core/style/content_data.h
@@ -50,6 +50,7 @@
virtual bool IsQuote() const { return false; }
virtual bool IsText() const { return false; }
virtual bool IsAltText() const { return false; }
+ virtual bool IsNone() const { return false; }
virtual LayoutObject* CreateLayoutObject(PseudoElement&,
const ComputedStyle&,
@@ -58,7 +59,11 @@
virtual ContentData* Clone() const;
ContentData* Next() const { return next_.Get(); }
- void SetNext(ContentData* next) { next_ = next; }
+ void SetNext(ContentData* next) {
+ DCHECK(!IsNone());
+ DCHECK(!next || !next->IsNone());
+ next_ = next;
+ }
virtual bool Equals(const ContentData&) const = 0;
@@ -258,6 +263,30 @@
}
};
+class NoneContentData final : public ContentData {
+ friend class ContentData;
+
+ public:
+ explicit NoneContentData() {}
+
+ bool IsNone() const override { return true; }
+ LayoutObject* CreateLayoutObject(PseudoElement&,
+ const ComputedStyle&,
+ LegacyLayout) const override;
+
+ bool Equals(const ContentData& data) const override { return IsNone(); }
+
+ private:
+ ContentData* CloneInternal() const override {
+ return MakeGarbageCollected<NoneContentData>();
+ }
+};
+
+template <>
+struct DowncastTraits<NoneContentData> {
+ static bool AllowFrom(const ContentData& content) { return content.IsNone(); }
+};
+
inline bool operator==(const ContentData& a, const ContentData& b) {
const ContentData* ptr_a = &a;
const ContentData* ptr_b = &b;
diff --git a/third_party/blink/renderer/modules/accessibility/ax_node_object.cc b/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
index d980f6c..2e4703c 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
+++ b/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
@@ -307,7 +307,7 @@
base::Optional<String> AXNodeObject::GetCSSAltText(Node* node) {
if (!node || !node->GetComputedStyle() ||
- !node->GetComputedStyle()->GetContentData()) {
+ node->GetComputedStyle()->ContentBehavesAsNormal()) {
return base::nullopt;
}
diff --git a/third_party/blink/web_tests/FlagExpectations/disable-layout-ng b/third_party/blink/web_tests/FlagExpectations/disable-layout-ng
index 2415961c..49d9414 100644
--- a/third_party/blink/web_tests/FlagExpectations/disable-layout-ng
+++ b/third_party/blink/web_tests/FlagExpectations/disable-layout-ng
@@ -82,6 +82,7 @@
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/457718 external/wpt/css/css-pseudo/marker-content-019.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-computed-content.html b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-computed-content.html
new file mode 100644
index 0000000..b2df337
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-computed-content.html
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Pseudo-Elements Test: Computed size of ::marker</title>
+<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#marker-pseudo">
+<link rel="help" href="https://drafts.csswg.org/css-content/#content-property">
+<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
+<meta name="assert" content="This test checks that 'content' resolves correctly in ::marker." />
+<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+<style>
+.no-list > li {
+ display: block;
+}
+.normal::marker {
+ content: normal;
+}
+.string::marker {
+ content: "string";
+}
+.image::marker {
+ content: url("about:invalid");
+}
+.none::marker {
+ content: none;
+}
+</style>
+<div id="log"></div>
+<ol class="list">
+ <li class="default">item</li>
+ <li class="normal">item</li>
+ <li class="string">item</li>
+ <li class="image">item</li>
+ <li class="none">item</li>
+</ol>
+<ol class="no-list">
+ <li class="default">item</li>
+ <li class="normal">item</li>
+ <li class="string">item</li>
+ <li class="image">item</li>
+ <li class="none">item</li>
+</ol>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+const expectations = {
+ default: 'normal',
+ normal: 'normal',
+ string: '"string"',
+ image: 'url("about:invalid")',
+ none: 'none',
+};
+for (const target of document.querySelectorAll('.list > li')) {
+ const {content} = getComputedStyle(target, '::marker');
+ test(() => {
+ debugger;
+ assert_equals(content, expectations[target.className]);
+ }, `Computed 'content' for list-item ::marker, variant ${target.className}`);
+}
+for (const target of document.querySelectorAll('.no-list > li')) {
+ const {content} = getComputedStyle(target, '::marker');
+ test(() => {
+ assert_equals(content, expectations[target.className]);
+ }, `Computed 'content' for non-list-item ::marker, variant ${target.className}`);
+}
+</script>
diff --git a/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-computed-size-expected.txt b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-computed-size-expected.txt
deleted file mode 100644
index 6ddc9e6..0000000
--- a/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-computed-size-expected.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-This is a testharness.js-based test.
-PASS Decimal ::marker
-PASS Decimal ::marker with custom value
-PASS String ::marker
-PASS ::marker with no box due to 'list-style'
-PASS ::marker with custom string contents
-PASS ::marker with custom image contents
-PASS ::marker with custom string and image contents
-FAIL ::marker with no box due to 'content' assert_equals: width expected "auto" but got "5px"
-Harness: the test ran to completion.
-
diff --git a/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-019-ref.html b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-019-ref.html
new file mode 100644
index 0000000..4b96577
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-019-ref.html
@@ -0,0 +1,21 @@
+<!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>
+li {
+ display: block;
+}
+</style>
+<ol>
+ <li>inside symbol</li>
+ <li>inside decimal</li>
+ <li>inside string</li>
+ <li>inside image</li>
+</ol>
+<ol>
+ <li>outside symbol</li>
+ <li>outside decimal</li>
+ <li>outside string</li>
+ <li>outside image</li>
+</ol>
diff --git a/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-019.html b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-019.html
new file mode 100644
index 0000000..4921ca7
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/css-pseudo/marker-content-019.html
@@ -0,0 +1,40 @@
+<!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-019-ref.html">
+<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#marker-pseudo">
+<link rel="help" href="https://drafts.csswg.org/css-content/#content-property">
+<meta name="assert" content="Checks that the 'content: none' hides the ::marker.">
+<style>
+::marker {
+ content: none
+}
+.inside {
+ list-style-position: inside;
+}
+.symbol {
+ list-style-type: disc;
+}
+.decimal {
+ list-style-type: decimal;
+}
+.string {
+ list-style-type: "string";
+}
+.image {
+ list-style-image: url("/images/green-100x50.png");
+}
+</style>
+<ol class="inside">
+ <li class="symbol">inside symbol</li>
+ <li class="decimal">inside decimal</li>
+ <li class="string">inside string</li>
+ <li class="image">inside image</li>
+</ol>
+<ol class="outside">
+ <li class="symbol">outside symbol</li>
+ <li class="decimal">outside decimal</li>
+ <li class="string">outside string</li>
+ <li class="image">outside image</li>
+</ol>
