Skip to content

Commit

Permalink
Merge r245158 - Do not mix inline and block level boxes.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=197462
<rdar://problem/50369362>

Reviewed by Antti Koivisto.

Source/WebCore:

This patch tightens the remove-anonymous-wrappers logic by checking if the removal would
produce an inline-block sibling mix.
When a block level box is removed from the tree, we check if after the removal the anonymous sibling block
boxes are still needed or whether we can removed them as well (and have only inline level child boxes).
In addition to checking if the container is anonymous and is part of a continuation, we also need to check
if collapsing it (and by that moving its children one level up) would cause a inline-block box mix.

Test: fast/ruby/continuation-and-column-spanner-crash.html

* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::removeAnonymousWrappersForInlineChildrenIfNeeded):
* rendering/updating/RenderTreeBuilderContinuation.cpp:
(WebCore::RenderTreeBuilder::Continuation::cleanupOnDestroy):

LayoutTests:

* fast/ruby/continuation-and-column-spanner-crash-expected.txt: Added.
* fast/ruby/continuation-and-column-spanner-crash.html: Added.
  • Loading branch information
alanbaradlay authored and mcatanzaro committed Aug 4, 2019
1 parent 36298f1 commit 7e0c908
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 9 deletions.
11 changes: 11 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
2019-05-08 Zalan Bujtas <zalan@apple.com>

Do not mix inline and block level boxes.
https://bugs.webkit.org/show_bug.cgi?id=197462
<rdar://problem/50369362>

Reviewed by Antti Koivisto.

* fast/ruby/continuation-and-column-spanner-crash-expected.txt: Added.
* fast/ruby/continuation-and-column-spanner-crash.html: Added.

2019-05-04 Tadeu Zagallo <tzagallo@apple.com>

TypedArrays should not store properties that are canonical numeric indices
Expand Down
@@ -0,0 +1,2 @@
PASS if no crash or assert.

10 changes: 10 additions & 0 deletions LayoutTests/fast/ruby/continuation-and-column-spanner-crash.html
@@ -0,0 +1,10 @@
PASS if no crash or assert.
<ruby><rtc><span><details open="false"><span id=span2></span></details></span><div id=div3></div></rtc><rt id=rt2></rt></ruby><script>
document.body.offsetHeight;
rt2.remove();
span2.remove();
document.body.offsetHeight;
div3.style.cssText = "column-span: all";
if (window.testRunner)
testRunner.dumpAsText();
</script>
22 changes: 22 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
2019-05-09 Zalan Bujtas <zalan@apple.com>

Do not mix inline and block level boxes.
https://bugs.webkit.org/show_bug.cgi?id=197462
<rdar://problem/50369362>

Reviewed by Antti Koivisto.

This patch tightens the remove-anonymous-wrappers logic by checking if the removal would
produce an inline-block sibling mix.
When a block level box is removed from the tree, we check if after the removal the anonymous sibling block
boxes are still needed or whether we can removed them as well (and have only inline level child boxes).
In addition to checking if the container is anonymous and is part of a continuation, we also need to check
if collapsing it (and by that moving its children one level up) would cause a inline-block box mix.

Test: fast/ruby/continuation-and-column-spanner-crash.html

* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::removeAnonymousWrappersForInlineChildrenIfNeeded):
* rendering/updating/RenderTreeBuilderContinuation.cpp:
(WebCore::RenderTreeBuilder::Continuation::cleanupOnDestroy):

2019-03-20 Michael Catanzaro <mcatanzaro@igalia.com>

Remove copyRef() calls added in r243163
Expand Down
29 changes: 21 additions & 8 deletions Source/WebCore/rendering/updating/RenderTreeBuilder.cpp
Expand Up @@ -683,15 +683,28 @@ void RenderTreeBuilder::removeAnonymousWrappersForInlineChildrenIfNeeded(RenderE
// otherwise we can proceed to stripping solitary anonymous wrappers from the inlines.
// FIXME: We should also handle split inlines here - we exclude them at the moment by returning
// if we find a continuation.
auto* current = blockParent.firstChild();
while (current && ((current->isAnonymousBlock() && !downcast<RenderBlock>(*current).isContinuation()) || current->style().isFloating() || current->style().hasOutOfFlowPosition()))
current = current->nextSibling();

if (current)
return;
Optional<bool> shouldAllChildrenBeInline;
for (auto* current = blockParent.firstChild(); current; current = current->nextSibling()) {
if (current->style().isFloating() || current->style().hasOutOfFlowPosition())
continue;
if (!current->isAnonymousBlock() || downcast<RenderBlock>(*current).isContinuation())
return;
// Anonymous block not in continuation. Check if it holds a set of inline or block children and try not to mix them.
auto* firstChild = current->firstChildSlow();
if (!firstChild)
continue;
auto isInlineLevelBox = firstChild->isInline();
if (!shouldAllChildrenBeInline.hasValue()) {
shouldAllChildrenBeInline = isInlineLevelBox;
continue;
}
// Mixing inline and block level boxes?
if (*shouldAllChildrenBeInline != isInlineLevelBox)
return;
}

RenderObject* next;
for (current = blockParent.firstChild(); current; current = next) {
RenderObject* next = nullptr;
for (auto* current = blockParent.firstChild(); current; current = next) {
next = current->nextSibling();
if (current->isAnonymousBlock())
blockBuilder().dropAnonymousBoxChild(blockParent, downcast<RenderBlock>(*current));
Expand Down
Expand Up @@ -37,8 +37,11 @@ RenderTreeBuilder::Continuation::Continuation(RenderTreeBuilder& builder)

void RenderTreeBuilder::Continuation::cleanupOnDestroy(RenderBoxModelObject& renderer)
{
if (!renderer.continuation() || renderer.isContinuation())
if (!renderer.continuation() || renderer.isContinuation()) {
if (renderer.hasContinuationChainNode())
renderer.removeFromContinuationChain();
return;
}

ASSERT(renderer.hasContinuationChainNode());
ASSERT(renderer.continuationChainNode());
Expand Down

0 comments on commit 7e0c908

Please sign in to comment.