Skip to content

GitLab

T
tangled
Commit 62649b86 authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Commit Bot
Browse files
Options

Find LayoutText containing the first letter before attaching pseudo. 

We assert that we find the same text node before creating the element as
we find when attaching the first letter LayoutText. We used to attach
the ::first-letter pseudo element before finding the LayoutText from
which we get the first letter text.

We did crash in a clusterfuzz test because FirstLetterTextLayoutObject()
was confused by a combination of a grid, button, anonymous
wrappers and continuations. Instead of trying to fix all bugs in
FirstLetterTextLayoutObject(), find the LayoutText before attaching the
::first-letter to make sure we are consistent instead of chasing
clusterfuzz issues.

Reported the incorrectness for button, grid, and ::first-letter in
868380.

Bug: 859285, 868380
Change-Id: I335a32b466ab31858fb05ea5f650cf12ab674040
Reviewed-on: https://chromium-review.googlesource.com/1152982Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578993}
parent b904d324
Hide whitespace changes
Inline Side-by-side
Showing with 41 additions and 19 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 62649b86
...@@ -3650,6 +3650,7 @@ crbug.com/697971 [ Mac10.12 ] fast/text/selection/flexbox-selection.html [ Skip ...@@ -3650,6 +3650,7 @@ crbug.com/697971 [ Mac10.12 ] fast/text/selection/flexbox-selection.html [ Skip
crbug.com/659610 fast/css-grid-layout/grid-baseline.html [ Failure ] crbug.com/659610 fast/css-grid-layout/grid-baseline.html [ Failure ]
crbug.com/659610 fast/css-grid-layout/grid-baseline-margins.html [ Failure ] crbug.com/659610 fast/css-grid-layout/grid-baseline-margins.html [ Failure ]
crbug.com/511177 external/wpt/css/css-grid/grid-layout-properties.html [ Failure ] crbug.com/511177 external/wpt/css/css-grid/grid-layout-properties.html [ Failure ]
crbug.com/868380 external/wpt/css/css-grid/grid-model/grid-container-ignores-first-letter-002.html [ Failure ]
crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-align-baseline-vertical.html [ Failure ] crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-align-baseline-vertical.html [ Failure ]
crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-self-baseline-03.html [ Failure ] crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-self-baseline-03.html [ Failure ]
crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-self-baseline-04.html [ Failure ] crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-self-baseline-04.html [ Failure ]
......
third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-ignores-first-letter-002-ref.html 0 → 100644
View file @ 62649b86
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test Reference</title>
<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
<button style="display:grid">
First letter should not be red
</button>
third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-ignores-first-letter-002.html 0 → 100644
View file @ 62649b86
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: '::first-letter' is ignored in button grid containers</title>
<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
<link rel="help" href="http://www.w3.org/TR/css-grid-1/#grid-containers">
<link rel="help" href="http://www.w3.org/TR/css3-selectors/#first-letter">
<link rel="match" href="grid-container-ignores-first-letter-002-ref.html">
<style>
/* The combination of button, grid, nested inlines, and ::first-letter crashes Blink */
button { display: grid }
button::first-letter { color: red }
</style>
<button>
<span>
<span>First letter should not be red</span>
</span>
</button>
third_party/blink/renderer/core/dom/first_letter_pseudo_element.cc
View file @ 62649b86
...@@ -94,7 +94,7 @@ static bool IsInvalidFirstLetterLayoutObject(const LayoutObject* obj) { ...@@ -94,7 +94,7 @@ static bool IsInvalidFirstLetterLayoutObject(const LayoutObject* obj) {
return (obj->IsBR() || (obj->IsText() && ToLayoutText(obj)->IsWordBreak())); return (obj->IsBR() || (obj->IsText() && ToLayoutText(obj)->IsWordBreak()));
} }
LayoutObject* FirstLetterPseudoElement::FirstLetterTextLayoutObject( LayoutText* FirstLetterPseudoElement::FirstLetterTextLayoutObject(
const Element& element) { const Element& element) {
LayoutObject* parent_layout_object = nullptr; LayoutObject* parent_layout_object = nullptr;
...@@ -184,7 +184,7 @@ LayoutObject* FirstLetterPseudoElement::FirstLetterTextLayoutObject( ...@@ -184,7 +184,7 @@ LayoutObject* FirstLetterPseudoElement::FirstLetterTextLayoutObject(
IsInvalidFirstLetterLayoutObject(first_letter_text_layout_object)) IsInvalidFirstLetterLayoutObject(first_letter_text_layout_object))
return nullptr; return nullptr;
return first_letter_text_layout_object; return ToLayoutText(first_letter_text_layout_object);
} }
FirstLetterPseudoElement::FirstLetterPseudoElement(Element* parent) FirstLetterPseudoElement::FirstLetterPseudoElement(Element* parent)
...@@ -241,8 +241,10 @@ void FirstLetterPseudoElement::SetRemainingTextLayoutObject( ...@@ -241,8 +241,10 @@ void FirstLetterPseudoElement::SetRemainingTextLayoutObject(
} }
void FirstLetterPseudoElement::AttachLayoutTree(AttachContext& context) { void FirstLetterPseudoElement::AttachLayoutTree(AttachContext& context) {
LayoutText* first_letter_text =
FirstLetterPseudoElement::FirstLetterTextLayoutObject(*this);
PseudoElement::AttachLayoutTree(context); PseudoElement::AttachLayoutTree(context);
AttachFirstLetterTextLayoutObjects(); AttachFirstLetterTextLayoutObjects(first_letter_text);
} }
void FirstLetterPseudoElement::DetachLayoutTree(const AttachContext& context) { void FirstLetterPseudoElement::DetachLayoutTree(const AttachContext& context) {
...@@ -283,26 +285,20 @@ ComputedStyle* FirstLetterPseudoElement::StyleForFirstLetter( ...@@ -283,26 +285,20 @@ ComputedStyle* FirstLetterPseudoElement::StyleForFirstLetter(
return pseudo_style; return pseudo_style;
} }
void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects() { void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(LayoutText* first_letter_text) {
LayoutObject* next_layout_object = DCHECK(first_letter_text);
FirstLetterPseudoElement::FirstLetterTextLayoutObject(*this);
DCHECK(next_layout_object);
DCHECK(next_layout_object->IsText());
// The original string is going to be either a generated content string or a // The original string is going to be either a generated content string or a
// DOM node's string. We want the original string before it got transformed in // DOM node's string. We want the original string before it got transformed in
// case first-letter has no text-transform or a different text-transform // case first-letter has no text-transform or a different text-transform
// applied to it. // applied to it.
String old_text = String old_text = first_letter_text->IsTextFragment() ? ToLayoutTextFragment(first_letter_text)->CompleteText() : first_letter_text->OriginalText();
ToLayoutText(next_layout_object)->IsTextFragment()
? ToLayoutTextFragment(next_layout_object)->CompleteText()
: ToLayoutText(next_layout_object)->OriginalText();
DCHECK(old_text.Impl()); DCHECK(old_text.Impl());
// :first-letter inherits from the parent of the text. It may not be // :first-letter inherits from the parent of the text. It may not be
// this->Parent() when e.g., <div><span>text</span></div>. // this->Parent() when e.g., <div><span>text</span></div>.
ComputedStyle* pseudo_style = ComputedStyle* pseudo_style =
StyleForFirstLetter(next_layout_object->Parent()); StyleForFirstLetter(first_letter_text->Parent());
GetLayoutObject()->SetStyle(pseudo_style); GetLayoutObject()->SetStyle(pseudo_style);
// FIXME: This would already have been calculated in firstLetterLayoutObject. // FIXME: This would already have been calculated in firstLetterLayoutObject.
...@@ -314,9 +310,9 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects() { ...@@ -314,9 +310,9 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects() {
// This text fragment might be empty. // This text fragment might be empty.
LayoutTextFragment* remaining_text; LayoutTextFragment* remaining_text;
if (next_layout_object->GetNode()) { if (first_letter_text->GetNode()) {
remaining_text = remaining_text =
new LayoutTextFragment(next_layout_object->GetNode(), old_text.Impl(), new LayoutTextFragment(first_letter_text->GetNode(), old_text.Impl(),
length, remaining_length); length, remaining_length);
} else { } else {
remaining_text = LayoutTextFragment::CreateAnonymous( remaining_text = LayoutTextFragment::CreateAnonymous(
...@@ -325,7 +321,7 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects() { ...@@ -325,7 +321,7 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects() {
remaining_text->SetFirstLetterPseudoElement(this); remaining_text->SetFirstLetterPseudoElement(this);
remaining_text->SetIsRemainingTextLayoutObject(true); remaining_text->SetIsRemainingTextLayoutObject(true);
remaining_text->SetStyle(next_layout_object->MutableStyle()); remaining_text->SetStyle(first_letter_text->MutableStyle());
if (remaining_text->GetNode()) if (remaining_text->GetNode())
remaining_text->GetNode()->SetLayoutObject(remaining_text); remaining_text->GetNode()->SetLayoutObject(remaining_text);
...@@ -342,7 +338,7 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects() { ...@@ -342,7 +338,7 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects() {
letter->SetStyle(pseudo_style); letter->SetStyle(pseudo_style);
GetLayoutObject()->AddChild(letter); GetLayoutObject()->AddChild(letter);
next_layout_object->Destroy(); first_letter_text->Destroy();
} }
void FirstLetterPseudoElement::DidRecalcStyle(StyleRecalcChange) { void FirstLetterPseudoElement::DidRecalcStyle(StyleRecalcChange) {
......
third_party/blink/renderer/core/dom/first_letter_pseudo_element.h
View file @ 62649b86
...@@ -34,6 +34,7 @@ namespace blink { ...@@ -34,6 +34,7 @@ namespace blink {
class Element; class Element;
class LayoutObject; class LayoutObject;
class LayoutText;
class LayoutTextFragment; class LayoutTextFragment;
class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement { class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement {
...@@ -44,7 +45,7 @@ class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement { ...@@ -44,7 +45,7 @@ class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement {
~FirstLetterPseudoElement() override; ~FirstLetterPseudoElement() override;
static LayoutObject* FirstLetterTextLayoutObject(const Element&); static LayoutText* FirstLetterTextLayoutObject(const Element&);
static unsigned FirstLetterLength(const String&); static unsigned FirstLetterLength(const String&);
void SetRemainingTextLayoutObject(LayoutTextFragment*); void SetRemainingTextLayoutObject(LayoutTextFragment*);
...@@ -62,7 +63,7 @@ class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement { ...@@ -62,7 +63,7 @@ class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement {
void DidRecalcStyle(StyleRecalcChange) override; void DidRecalcStyle(StyleRecalcChange) override;
void AttachFirstLetterTextLayoutObjects(); void AttachFirstLetterTextLayoutObjects(LayoutText* first_letter_text);
ComputedStyle* StyleForFirstLetter(LayoutObject*); ComputedStyle* StyleForFirstLetter(LayoutObject*);
LayoutTextFragment* remaining_text_layout_object_; LayoutTextFragment* remaining_text_layout_object_;
......
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