Skip to content

GitLab

T
tangled
Commit 046e4ea7 authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Commit Bot
Browse files
Options

Don't do whitespace re-attachment for ::first-letter. 

If we have a white-space node before the text node with the first
letter, we would try to re-attach the white-space node with an incorrect
previous in-flow LayoutObject because we attach the ::first-letter after
all children have been attached, not where the ::first-letter would end
up in the layout tree.

We don't need to re-attach whitespace after ::first-letter because:

* We always create a remaining LayoutTextFragment for the text node
  which contains the first formatted letter. Wether the remaining text
  consists of white-space or is even empty.

* If the LayoutObject sibling following first-letter+remaining-text
  consists of white-space, it would have gotten a LayoutObject when
  attaching the LayoutText for the text node where the first-letter
  comes from.

That is, we will always have a LayoutObject for a white-space LayoutText
sibling of a first-letter LayoutObject.

Split out a separate method for ::first-letter for less complexity and
cost with which WhitespaceAttacher to use.

Bug: 876150
Change-Id: Ib1f34f0ba596d5fa5f1c7fcbd7c75550dd4f9d12
Reviewed-on: https://chromium-review.googlesource.com/1183231Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584866}
parent 13c54c9e
Hide whitespace changes
Inline Side-by-side
Showing with 69 additions and 20 deletions
third_party/WebKit/LayoutTests/external/wpt/css/css-pseudo/first-letter-and-whitespace-ref.html 0 → 100644
View file @ 046e4ea7
<!doctype html>
<meta charset="utf-8">
<title>CSS Reference File</title>
<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
<p>Pass if no space between "A", space between "B".</p>
<div><span style="float:left">A</span>A</div>
<div><span style="float:left">A</span>A</div>
<div>B B</div>
<div>B B</div>
<div>AAB B</div>
<div>AAB B</div>
third_party/WebKit/LayoutTests/external/wpt/css/css-pseudo/first-letter-and-whitespace.html 0 → 100644
View file @ 046e4ea7
<!doctype html>
<meta charset="utf-8">
<title>CSS Test: White-spaces around floated ::first-letter</title>
<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
<link rel="match" href="first-letter-and-whitespace-ref.html">
<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#first-letter-styling">
<meta name="assert" content="Test checks that white-spaces are correctly rendered for floated ::first-letter changes">
<style>
div::first-letter {
color: black;
}
.floatLetter::first-letter {
float: left;
}
</style>
<p>Pass if no space between "A", space between "B".</p>
<div id="t1"> <!---->AA</div>
<div id="t2"> <!---->A<!----> <!---->A</div>
<div id="t3" class="floatLetter"> <!---->B<!----> <!---->B</div>
<div id="t4" class="floatLetter"> <!---->B <!---->B</div>
<div id="t5" class="floatLetter"> <!---->AAB<!----> <!---->B</div>
<div id="t6" class="floatLetter"> <!---->AAB<!----> B</div>
<script>
document.body.offsetTop;
t1.className = "floatLetter";
t2.className = "floatLetter";
t3.className = "";
t4.className = "";
t5.className = "";
t6.className = "";
</script>
third_party/WebKit/LayoutTests/fast/css-generated-content/first-letter-next-sibling-crash-expected.txt
View file @ 046e4ea7
WebKit Bug 85759 - Crash in LayoutBlockFlow::updateFirstLetterStyle.
PASS if test does not crash.
PASS if test does not crash.
third_party/blink/renderer/core/dom/element.cc
View file @ 046e4ea7
......@@ -2577,7 +2577,7 @@ void Element::RebuildLayoutTree(WhitespaceAttacher& whitespace_attacher) {
RebuildChildrenLayoutTrees(*child_attacher);
RebuildPseudoElementLayoutTree(kPseudoIdBefore, *child_attacher);
RebuildPseudoElementLayoutTree(kPseudoIdBackdrop, *child_attacher);
RebuildPseudoElementLayoutTree(kPseudoIdFirstLetter, *child_attacher);
RebuildFirstLetterLayoutTree();
ClearChildNeedsReattachLayoutTree();
}
DCHECK(!NeedsStyleRecalc());
......@@ -2598,29 +2598,35 @@ void Element::RebuildShadowRootLayoutTree(
void Element::RebuildPseudoElementLayoutTree(
PseudoId pseudo_id,
WhitespaceAttacher& whitespace_attacher) {
if (pseudo_id == kPseudoIdFirstLetter) {
// Need to create a ::first-letter element here for the following case:
//
// <style>#outer::first-letter {...}</style>
// <div id=outer><div id=inner style="display:none">Text</div></div>
// <script> outer.offsetTop; inner.style.display = "block" </script>
//
// The creation of FirstLetterPseudoElement relies on the layout tree of the
// block contents. In this case, the ::first-letter element is not created
// initially since the #inner div is not displayed. On RecalcStyle it's not
// created since the layout tree is still not built, and AttachLayoutTree
// for #inner will not update the ::first-letter of outer. However, we end
// up here for #outer after AttachLayoutTree is called on #inner at which
// point the layout sub-tree is available for deciding on creating the
// ::first-letter.
UpdateFirstLetterPseudoElement(StyleUpdatePhase::kRebuildLayoutTree);
}
if (PseudoElement* element = GetPseudoElement(pseudo_id)) {
if (element->NeedsRebuildLayoutTree(whitespace_attacher))
element->RebuildLayoutTree(whitespace_attacher);
}
}
void Element::RebuildFirstLetterLayoutTree() {
// Need to create a ::first-letter element here for the following case:
//
// <style>#outer::first-letter {...}</style>
// <div id=outer><div id=inner style="display:none">Text</div></div>
// <script> outer.offsetTop; inner.style.display = "block" </script>
//
// The creation of FirstLetterPseudoElement relies on the layout tree of the
// block contents. In this case, the ::first-letter element is not created
// initially since the #inner div is not displayed. On RecalcStyle it's not
// created since the layout tree is still not built, and AttachLayoutTree
// for #inner will not update the ::first-letter of outer. However, we end
// up here for #outer after AttachLayoutTree is called on #inner at which
// point the layout sub-tree is available for deciding on creating the
// ::first-letter.
UpdateFirstLetterPseudoElement(StyleUpdatePhase::kRebuildLayoutTree);
if (PseudoElement* element = GetPseudoElement(kPseudoIdFirstLetter)) {
WhitespaceAttacher whitespace_attacher;
if (element->NeedsRebuildLayoutTree(whitespace_attacher))
element->RebuildLayoutTree(whitespace_attacher);
}
}
void Element::UpdateCallbackSelectors(const ComputedStyle* old_style,
const ComputedStyle* new_style) {
Vector<String> empty_vector;
......
third_party/blink/renderer/core/dom/element.h
View file @ 046e4ea7
......@@ -989,6 +989,7 @@ class CORE_EXPORT Element : public ContainerNode {
bool ShouldCallRecalcStyleForChildren(StyleRecalcChange);
void RebuildPseudoElementLayoutTree(PseudoId, WhitespaceAttacher&);
void RebuildFirstLetterLayoutTree();
void RebuildShadowRootLayoutTree(WhitespaceAttacher&);
inline void CheckForEmptyStyleChange(const Node* node_before_change,
const Node* node_after_change);
......
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