Commit 6d4c379c authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

Cope even better with the mess we leave behind after a continuation.

Tried to fix this in https://chromium-review.googlesource.com/c/1309777
but incorrectly assumed that bogus continuation chains were formed by
direct siblings. But they may also be arbitrary-level cousins. Need to
walk through all non-atomic inlines between one continuation object and
the next in the chain, to figure out whether there is an anonymous block
between two objects in the continuation chain. Only then should we
update the positionedness of anonymous blocks. Only perform this check
if positionedness changed, since it's not for free.

Bug: 901598
Change-Id: I3e49a4ce8081a4a3190cc1569480189cbcccdcc4
Reviewed-on: https://chromium-review.googlesource.com/c/1317627Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605465}
parent 0d018b46
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/CSS22/visudet.html#containing-block-details">
<link rel="help" href="https://www.w3.org/TR/CSS22/visuren.html#anonymous-block-level">
<p>There should be a green square below, and no red.</p>
<div style="position:relative; width:100px; height:100px; background:red;">
<span>
<span id="posMe">
<div id="removeMe"></div>
</span>
</span>
<span>
<div>
<div id="target" style="position:absolute; width:100%; height:100%; background:green;"></div>
</div>
</span>
</div>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
test(()=> {
document.body.offsetTop;
removeMe.style.display = "none";
document.body.offsetTop;
posMe.style.position = "relative";
assert_equals(document.getElementById("target").offsetWidth, 100);
assert_equals(document.getElementById("target").offsetHeight, 100);
assert_equals(document.getElementById("target").offsetLeft, 0);
assert_equals(document.getElementById("target").offsetTop, 0);
}, "Make sure that we're sized by the right ancestor");
</script>
...@@ -231,25 +231,50 @@ void LayoutInline::StyleDidChange(StyleDifference diff, ...@@ -231,25 +231,50 @@ void LayoutInline::StyleDidChange(StyleDifference diff,
// to pass its style on to anyone else. // to pass its style on to anyone else.
const ComputedStyle& new_style = StyleRef(); const ComputedStyle& new_style = StyleRef();
if (LayoutInline* continuation = InlineElementContinuation()) { if (LayoutInline* continuation = InlineElementContinuation()) {
bool position_type_changed =
old_style && (new_style.GetPosition() != old_style->GetPosition()) &&
(new_style.HasInFlowPosition() || old_style->HasInFlowPosition());
// An inline that establishes a continuation is normally wrapped inside an
// anonymous block, of course, but if the continuation chain has been
// partially torn down (read comment further below), this is not the
// case. Here we want to find the nearest containing block that's an
// ancestor of all the continuations.
const LayoutBlock* boundary = ContainingBlock();
if (boundary->IsAnonymousBlock())
boundary = boundary->ContainingBlock();
LayoutInline* previous_part = this; LayoutInline* previous_part = this;
LayoutInline* end_of_continuation = nullptr; LayoutInline* end_of_continuation = nullptr;
bool is_real_continuation = false; bool needs_anon_block_position_update = false;
for (LayoutInline* curr_cont = continuation; curr_cont; for (LayoutInline* curr_cont = continuation; curr_cont;
curr_cont = curr_cont->InlineElementContinuation()) { curr_cont = curr_cont->InlineElementContinuation()) {
if (!is_real_continuation && curr_cont != previous_part->NextSibling()) { if (position_type_changed && !needs_anon_block_position_update) {
// When we have a continuation chain, and the block child that was the // When we have a continuation chain, and the block child that was the
// reason for creating that in the first place is removed, we don't // reason for creating that in the first place is removed, we don't
// clean up the continuation chain. Previously split inlines should // clean up the continuation chain. Previously split inlines should
// ideally be joined when this happens, but we don't do that, but rather // ideally be joined when this happens, but we don't do that, but rather
// leave the mess around. We'll end up with direct LayoutInline siblings // leave the mess around. We'll end up with LayoutInline siblings or
// for the same Node (that form a bogus continuation chain). Here we // cousins for the same Node (that form a bogus continuation chain),
// check that we're NOT in such a situation, and that the Node actually // without any blocks in-between. Here we check that we're NOT in such a
// forms a real continuation chain. This matters when it comes to // situation, and that the Node actually forms a real continuation
// marking the anonymous container(s) of block children as relatively // chain. This matters when it comes to marking the anonymous
// positioned. We should only do that if the Node is an ancestor of the // container(s) of block children as relatively positioned (further
// below). We should only do that if the Node is an ancestor of the
// blocks. Otherwise out-of-flow positioned descendants will use the // blocks. Otherwise out-of-flow positioned descendants will use the
// wrong containing block. // wrong containing block.
is_real_continuation = true; for (LayoutObject* walker = previous_part->NextInPreOrder(boundary);
walker;) {
if (walker == curr_cont)
break;
if (walker->IsAnonymousBlock()) {
needs_anon_block_position_update = true;
break;
}
if (walker->IsLayoutInline())
walker = walker->NextInPreOrder(boundary);
else
walker = walker->NextInPreOrderAfterChildren(boundary);
}
} }
previous_part = curr_cont; previous_part = curr_cont;
...@@ -260,14 +285,13 @@ void LayoutInline::StyleDidChange(StyleDifference diff, ...@@ -260,14 +285,13 @@ void LayoutInline::StyleDidChange(StyleDifference diff,
end_of_continuation = curr_cont; end_of_continuation = curr_cont;
} }
if (is_real_continuation && old_style) { if (needs_anon_block_position_update) {
DCHECK(old_style);
DCHECK(end_of_continuation); DCHECK(end_of_continuation);
LayoutObject* block = ContainingBlock()->NextSibling(); LayoutObject* block = ContainingBlock()->NextSibling();
// If an inline's in-flow positioning has changed then any descendant // If an inline's in-flow positioning has changed then any descendant
// blocks will need to change their styles accordingly. // blocks will need to change their styles accordingly.
if (block && block->IsAnonymousBlock() && if (block && block->IsAnonymousBlock()) {
new_style.GetPosition() != old_style->GetPosition() &&
(new_style.HasInFlowPosition() || old_style->HasInFlowPosition())) {
UpdateInFlowPositionOfAnonymousBlockContinuations( UpdateInFlowPositionOfAnonymousBlockContinuations(
block, new_style, *old_style, block, new_style, *old_style,
end_of_continuation->ContainingBlock()); end_of_continuation->ContainingBlock());
......
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