From 2b4df29f211c7d48a2b73ce4c7ade8116a2336a4 Mon Sep 17 00:00:00 2001 From: "commit-queue@webkit.org" Date: Tue, 24 Jan 2012 23:53:27 +0000 Subject: [PATCH] Crash in updateFirstLetter() from unnecessary anonymous block https://bugs.webkit.org/show_bug.cgi?id=72675 Patch by Ken Buchanan on 2012-01-24 Reviewed by David Hyatt. Source/WebCore: There was a problem with anonymous blocks not getting removed when their only block flow siblings are removed if they also have non-block flow first-letter siblings (i.e. floats). This patch modifies RenderBlock::removeChild() to look for this situation and strip out unnecessary anonymous container blocks if it occurs. * rendering/RenderBlock.cpp: (WebCore::RenderBlock::removeChild): (WebCore::RenderBlock::collapseAnonymousBoxChild): Added * rendering/RenderBlock.h: (WebCore::RenderBlock::collapseAnonymousBoxChild): Added LayoutTests: Adding a test that cause a div to be removed from between a floating first-letter block and its remaining text. If the anonymous block is removed as an immediate consequence of the div removal, this shouldn't crash. * fast/css-generated-content/float-first-letter-siblings-convert-to-inline-expected.txt: Added * fast/css-generated-content/float-first-letter-siblings-convert-to-inline.html: Added git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105828 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 15 ++++++++++ ...-letter-siblings-convert-to-inline-expected.txt | 1 + ...at-first-letter-siblings-convert-to-inline.html | 34 ++++++++++++++++++++++ Source/WebCore/ChangeLog | 19 ++++++++++++ Source/WebCore/rendering/RenderBlock.cpp | 29 +++++++++++++----- Source/WebCore/rendering/RenderBlock.h | 2 ++ 6 files changed, 93 insertions(+), 7 deletions(-) mode change 100644 => 100755 LayoutTests/ChangeLog create mode 100755 LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline-expected.txt create mode 100755 LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline.html mode change 100644 => 100755 Source/WebCore/ChangeLog mode change 100644 => 100755 Source/WebCore/rendering/RenderBlock.h diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog old mode 100644 new mode 100755 index 51b7484..44a3e66 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,18 @@ +2012-01-24 Ken Buchanan + + Crash in updateFirstLetter() from unnecessary anonymous block + https://bugs.webkit.org/show_bug.cgi?id=72675 + + Reviewed by David Hyatt. + + Adding a test that cause a div to be removed from between a floating + first-letter block and its remaining text. If the anonymous block is + removed as an immediate consequence of the div removal, this shouldn't + crash. + + * fast/css-generated-content/float-first-letter-siblings-convert-to-inline-expected.txt: Added + * fast/css-generated-content/float-first-letter-siblings-convert-to-inline.html: Added + 2012-01-24 Adam Barth Mark these tests as flaky. diff --git a/LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline-expected.txt b/LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline-expected.txt new file mode 100755 index 0000000..91145d1 --- /dev/null +++ b/LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline-expected.txt @@ -0,0 +1 @@ +PASS if no crash or assert diff --git a/LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline.html b/LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline.html new file mode 100755 index 0000000..9b05ac2 --- /dev/null +++ b/LayoutTests/fast/css-generated-content/float-first-letter-siblings-convert-to-inline.html @@ -0,0 +1,34 @@ + + +
+
+ PASS if no crash or assert +
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog old mode 100644 new mode 100755 index da07ff2..6c0b150 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,22 @@ +2012-01-24 Ken Buchanan + + Crash in updateFirstLetter() from unnecessary anonymous block + https://bugs.webkit.org/show_bug.cgi?id=72675 + + Reviewed by David Hyatt. + + There was a problem with anonymous blocks not getting removed when + their only block flow siblings are removed if they also have non-block + flow first-letter siblings (i.e. floats). This patch modifies + RenderBlock::removeChild() to look for this situation and strip out + unnecessary anonymous container blocks if it occurs. + + * rendering/RenderBlock.cpp: + (WebCore::RenderBlock::removeChild): + (WebCore::RenderBlock::collapseAnonymousBoxChild): Added + * rendering/RenderBlock.h: + (WebCore::RenderBlock::collapseAnonymousBoxChild): Added + 2012-01-24 Daniel Cheng [chromium] event.dataTransfer.types should not return "Text" or "URL" diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp index ba5b013..badbc14 100755 --- a/Source/WebCore/rendering/RenderBlock.cpp +++ b/Source/WebCore/rendering/RenderBlock.cpp @@ -1025,6 +1025,17 @@ static bool canMergeContiguousAnonymousBlocks(RenderObject* oldChild, RenderObje && prev->isAnonymousColumnSpanBlock() == next->isAnonymousColumnSpanBlock(); } +void RenderBlock::collapseAnonymousBoxChild(RenderBlock* parent, RenderObject* child) +{ + parent->setNeedsLayoutAndPrefWidthsRecalc(); + parent->setChildrenInline(child->childrenInline()); + RenderBlock* anonBlock = toRenderBlock(parent->children()->removeChildNode(parent, child, child->hasLayer())); + anonBlock->moveAllChildrenTo(parent, child->hasLayer()); + // Delete the now-empty block's lines and nuke it. + anonBlock->deleteLineBoxTree(); + anonBlock->destroy(); +} + void RenderBlock::removeChild(RenderObject* oldChild) { // If this child is a block, and if our previous and next siblings are @@ -1081,13 +1092,17 @@ void RenderBlock::removeChild(RenderObject* oldChild) // The removal has knocked us down to containing only a single anonymous // box. We can go ahead and pull the content right back up into our // box. - setNeedsLayoutAndPrefWidthsRecalc(); - setChildrenInline(child->childrenInline()); - RenderBlock* anonBlock = toRenderBlock(children()->removeChildNode(this, child, child->hasLayer())); - anonBlock->moveAllChildrenTo(this, child->hasLayer()); - // Delete the now-empty block's lines and nuke it. - anonBlock->deleteLineBoxTree(); - anonBlock->destroy(); + collapseAnonymousBoxChild(this, child); + } else if ((prev && prev->isAnonymousBlock()) || (next && next->isAnonymousBlock())) { + // It's possible that the removal has knocked us down to a single anonymous + // block with pseudo-style element siblings (e.g. first-letter). If these + // are floating or positioned, then we need to pull the content up also. + RenderBlock* anonBlock = toRenderBlock((prev && prev->isAnonymousBlock()) ? prev : next); + if ((anonBlock->previousSibling() || anonBlock->nextSibling()) + && (!anonBlock->previousSibling() || (anonBlock->previousSibling()->style()->styleType() != NOPSEUDO && anonBlock->previousSibling()->isFloatingOrPositioned())) + && (!anonBlock->nextSibling() || (anonBlock->nextSibling()->style()->styleType() != NOPSEUDO && anonBlock->nextSibling()->isFloatingOrPositioned()))) { + collapseAnonymousBoxChild(this, anonBlock); + } } if (!firstChild() && !documentBeingDestroyed()) { diff --git a/Source/WebCore/rendering/RenderBlock.h b/Source/WebCore/rendering/RenderBlock.h old mode 100644 new mode 100755 index 6506dc8..1db5dee --- a/Source/WebCore/rendering/RenderBlock.h +++ b/Source/WebCore/rendering/RenderBlock.h @@ -461,6 +461,8 @@ private: void makeChildrenNonInline(RenderObject* insertionPoint = 0); virtual void removeLeftoverAnonymousBlock(RenderBlock* child); + static void collapseAnonymousBoxChild(RenderBlock* parent, RenderObject* child); + virtual void dirtyLinesFromChangedChild(RenderObject* child) { m_lineBoxes.dirtyLinesFromChangedChild(this, child); } void addChildToContinuation(RenderObject* newChild, RenderObject* beforeChild); -- 2.7.4