Commit 1a47a235 authored by Ting-Yu Lin's avatar Ting-Yu Lin
Browse files

Bug 1421105 Part 5 - Fix anonymous -moz-column-span-wrapper block's style is...

Bug 1421105 Part 5 - Fix anonymous -moz-column-span-wrapper block's style is overridden after restyling. r=bzbarsky,emilio

The major change in this patch is ::-moz-column-span-wrapper blocks are no
longer linked into the continuation chains when they're created in
CreateColumnSpanSiblings(). We can do that because
::-moz-column-span-wrapper is an non-inheriting anon box, which doesn't need
to be restyled.

This prevents RestyleManager::ProcessPostTraversal or
nsIFrame::UpdateStyleOfOwnedChildFrame, which set the same style on all
continuations of the frame they are working with, from overriding the
::-moz-column-span-wrapper style.

GetNextContinuationWithSameStyle was deleted in bug 1447367. Delete the comment
in nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit() to avoid confusion.

This patch also adds another condition to reframe the multi-column container
in MaybeRecreateContainerForFrameRemoval(). That condition is when an
element has a "column-span:all" descendant, i.e. the element's frame has
"column-span:all" siblings (which is created by CreateColumnSpanSiblings).
The added test multicol-span-all-dynamic-remove-006.html will fail without
this patch.

Depends on D5212

Differential Revision: https://phabricator.services.mozilla.com/D9988

--HG--
extra : moz-landing-system : lando
parent d1f59457
......@@ -2009,11 +2009,11 @@ ServoRestyleState::AssertOwner(const ServoRestyleState& aParent) const
{
MOZ_ASSERT(mOwner);
MOZ_ASSERT(!mOwner->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW));
MOZ_ASSERT(!mOwner->IsColumnSpanInMulticolSubtree());
// We allow aParent.mOwner to be null, for cases when we're not starting at
// the root of the tree. We also allow aParent.mOwner to be somewhere up our
// expected owner chain not our immediate owner, which allows us creating long
// chains of ServoRestyleStates in some cases where it's just not worth it.
#ifdef DEBUG
if (aParent.mOwner) {
const nsIFrame* owner = ExpectedOwnerForChild(mOwner);
if (owner != aParent.mOwner) {
......@@ -2029,7 +2029,6 @@ ServoRestyleState::AssertOwner(const ServoRestyleState& aParent) const
MOZ_ASSERT(found, "Must have aParent.mOwner on our expected owner chain");
}
}
#endif
}
nsChangeHint
......@@ -2656,6 +2655,13 @@ RestyleManager::ProcessPostTraversal(
primaryFrame &&
primaryFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW);
// We need this because any column-spanner's parent frame is not its DOM
// parent's primary frame. We need some special check similar to out-of-flow
// frames.
const bool isColumnSpan =
primaryFrame &&
primaryFrame->IsColumnSpanInMulticolSubtree();
// Grab the change hint from Servo.
bool wasRestyled;
nsChangeHint changeHint =
......@@ -2683,8 +2689,11 @@ RestyleManager::ProcessPostTraversal(
maybeAnonBoxChild = primaryFrame->GetPlaceholderFrame();
} else {
maybeAnonBoxChild = primaryFrame;
changeHint = NS_RemoveSubsumedHints(
changeHint, aRestyleState.ChangesHandledFor(styleFrame));
// Do not subsume change hints for the column-spanner.
if (!isColumnSpan) {
changeHint = NS_RemoveSubsumedHints(
changeHint, aRestyleState.ChangesHandledFor(styleFrame));
}
}
// If the parent wasn't restyled, the styles of our anon box parents won't
......@@ -2739,7 +2748,7 @@ RestyleManager::ProcessPostTraversal(
Maybe<ServoRestyleState> thisFrameRestyleState;
if (styleFrame) {
auto type = isOutOfFlow
auto type = isOutOfFlow || isColumnSpan
? ServoRestyleState::Type::OutOfFlow
: ServoRestyleState::Type::InFlow;
......
......@@ -8735,14 +8735,29 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame)
MOZ_ASSERT(aFrame == aFrame->FirstContinuation(),
"aFrame not the result of GetPrimaryFrame()?");
nsIFrame* inFlowFrame =
(aFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) ?
aFrame->GetPlaceholderFrame() : aFrame;
MOZ_ASSERT(inFlowFrame, "How did that happen?");
MOZ_ASSERT(inFlowFrame == inFlowFrame->FirstContinuation(),
"placeholder for primary frame has previous continuations?");
nsIFrame* parent = inFlowFrame->GetParent();
if (aFrame->HasAnyStateBits(NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR)) {
nsIFrame* parent = aFrame->GetParent();
nsIFrame* grandparent = parent->GetParent();
MOZ_ASSERT(grandparent);
bool needsReframe =
// 1. Removing a column-span may lead to an empty
// ::-moz-column-span-wrapper.
aFrame->IsColumnSpan() ||
// 2. Removing the only child of a ::-moz-column-content whose
// ColumnSet parent has a previous column-span sibling requires
// 2. Removing a frame which has any column-span siblings may also
// lead to an empty ::-moz-column-span-wrapper subtree. The
// column-span siblings were the frame's children, but later become
// the frame's siblings after CreateColumnSpanSiblings().
aFrame->GetProperty(nsIFrame::HasColumnSpanSiblings()) ||
// 3. Removing the only child of a ::-moz-column-content, whose
// ColumnSet grandparent has a previous column-span sibling, requires
// reframing since we might connect the ColumnSet's next column-span
// sibling (if there's one). Note that this isn't actually needed if
// the ColumnSet is at the end of ColumnSetWrapper since we create
......@@ -8754,9 +8769,9 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame)
!aFrame->GetPrevSibling() && !aFrame->GetNextSibling() &&
// That ::-moz-column-content is the first column.
!parent->GetPrevInFlow() &&
// The ColumnSet containing ::-moz-column-set has a previous sibling
// that is a column-span.
parent->GetPrevContinuation());
// The ColumnSet grandparent has a previous sibling that is a
// column-span.
grandparent->GetPrevSibling());
if (needsReframe) {
nsIFrame* containingBlock = GetMultiColumnContainingBlockFor(aFrame);
......@@ -8796,14 +8811,6 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame)
return true;
}
nsIFrame* inFlowFrame =
(aFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) ?
aFrame->GetPlaceholderFrame() : aFrame;
MOZ_ASSERT(inFlowFrame, "How did that happen?");
MOZ_ASSERT(inFlowFrame == inFlowFrame->FirstContinuation(),
"placeholder for primary frame has previous continuations?");
nsIFrame* parent = inFlowFrame->GetParent();
if (parent && parent->IsDetailsFrame()) {
HTMLSummaryElement* summary =
HTMLSummaryElement::FromNode(aFrame->GetContent());
......@@ -11072,14 +11079,10 @@ nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState& aState,
//
// 3) ColumnSetFrames are linked together as continuations.
//
// 4) Those column-span wrappers are linked together alternately with the
// original block frame and the original block's continuations. That
// is, original block frame -> column-span wrapper -> original block
// frame's continuation -> column-span wrapper -> ...
//
// This is very similar to what we do with block-inside-inline
// splitting, except here we can use continuations rather than the
// IBSibling linkage since the frames are the same type.
// 4) Those column-span wrappers are *not* linked together with themselves nor
// with the original block frame. The continuation chain consists of the
// original block frame and the original block's continuations wrapping
// non-column-spans.
//
// For example, this HTML
// <div id="x" style="column-count: 2;">
......@@ -11120,7 +11123,7 @@ nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState& aState,
//
// ColumnSet linkage described in 3): B -> G -> Q
//
// Column-span linkage described in 4): C -> D -> H -> K -> R and I -> L -> S
// Block linkage described in 4): C -> H -> R and I -> S
//
nsBlockFrame* blockFrame = do_QueryFrame(*aNewFrame);
......@@ -11424,8 +11427,10 @@ nsCSSFrameConstructor::CreateColumnSpanSiblings(nsFrameConstructorState& aState,
ComputedStyle* const initialBlockStyle = aInitialBlock->Style();
nsContainerFrame* const parentFrame = aInitialBlock->GetParent();
aInitialBlock->SetProperty(nsIFrame::HasColumnSpanSiblings(), true);
nsFrameItems siblings;
nsContainerFrame* lastNewBlock = aInitialBlock;
nsContainerFrame* lastNonColumnSpanWrapper = aInitialBlock;
do {
MOZ_ASSERT(aChildList.NotEmpty(), "Why call this if child list is empty?");
MOZ_ASSERT(aChildList.FirstChild()->IsColumnSpan(),
......@@ -11448,8 +11453,6 @@ nsCSSFrameConstructor::CreateColumnSpanSiblings(nsFrameConstructorState& aState,
aState.ReparentAbsoluteItems(columnSpanWrapper);
}
lastNewBlock->SetNextContinuation(columnSpanWrapper);
columnSpanWrapper->SetPrevContinuation(lastNewBlock);
siblings.AddChild(columnSpanWrapper);
// Grab the consecutive non-column-span kids, and reparent them into a
......@@ -11470,11 +11473,11 @@ nsCSSFrameConstructor::CreateColumnSpanSiblings(nsFrameConstructorState& aState,
}
}
columnSpanWrapper->SetNextContinuation(nonColumnSpanWrapper);
nonColumnSpanWrapper->SetPrevContinuation(columnSpanWrapper);
lastNonColumnSpanWrapper->SetNextContinuation(nonColumnSpanWrapper);
nonColumnSpanWrapper->SetPrevContinuation(lastNonColumnSpanWrapper);
siblings.AddChild(nonColumnSpanWrapper);
lastNewBlock = nonColumnSpanWrapper;
lastNonColumnSpanWrapper = nonColumnSpanWrapper;
} while (aChildList.NotEmpty());
return siblings;
......@@ -11492,10 +11495,9 @@ nsCSSFrameConstructor::ConstructInline(nsFrameConstructorState& aState,
// contain the runs of blocks, inline frames with our style for the runs of
// inlines, and put all these frames, in order, into aFrameItems.
//
// When there are column-span blocks in a run of blocks, instead of
// creating an anonymous block to wrap them, we create multiple anonymous
// blocks that are continuations of each other, wrapping runs of
// non-column-spans and runs of column-spans.
// When there are column-span blocks in a run of blocks, instead of creating
// an anonymous block to wrap them, we create multiple anonymous blocks,
// wrapping runs of non-column-spans and runs of column-spans.
//
// We return the the first one. The whole setup is called an {ib}
// split; in what follows "frames in the split" refers to the anonymous blocks
......@@ -11515,8 +11517,9 @@ nsCSSFrameConstructor::ConstructInline(nsFrameConstructorState& aState,
//
// 4) The first and last frame in the split are always inlines.
//
// 5) The frames wrapping runs of non-column-spans and runs of
// column-spans are linked together by continuations.
// 5) The frames wrapping runs of non-column-spans are linked together as
// continuations. The frames wrapping runs of column-spans are *not*
// linked with each other nor with other non-column-span wrappers.
//
// 6) The first and last frame in the chains of blocks are always wrapping
// non-column-spans. Both of them are created even if they're empty.
......
......@@ -6,6 +6,8 @@
#include "ColumnSetWrapperFrame.h"
#include "nsContentUtils.h"
using namespace mozilla;
nsBlockFrame*
......@@ -58,27 +60,18 @@ ColumnSetWrapperFrame::AppendDirectlyOwnedAnonBoxes(nsTArray<OwnedAnonBox>& aRes
"Who set NS_FRAME_OWNS_ANON_BOXES on our continuations?");
// It's sufficient to append the first ColumnSet child, which is the first
// continuation of the directly owned anon boxes.
// continuation of all the other ColumnSets.
//
// We don't need to append -moz-column-span-wrapper children because
// they're non-inheriting anon boxes, and they cannot have any directly
// owned anon boxes nor generate any native anonymous content themselves.
// Thus, no need to restyle them. AssertColumnSpanWrapperSubtreeIsSane()
// asserts all the conditions above which allow us to skip appending
// -moz-column-span-wrappers.
nsIFrame* columnSet = PrincipalChildList().FirstChild();
MOZ_ASSERT(columnSet && columnSet->IsColumnSetFrame(),
"The first child should always be ColumnSet!");
aResult.AppendElement(OwnedAnonBox(columnSet));
#ifdef DEBUG
// All the other ColumnSets are the continuation of the first ColumnSet;
// the -moz-column-span-wrappers are the continuations of other blocks in
// -moz-column-span-contents.
for (nsIFrame* child : PrincipalChildList()) {
if (child == columnSet) {
// Skip testing the first child.
continue;
}
MOZ_ASSERT((child->IsColumnSetFrame() ||
child->Style()->GetPseudo() == nsCSSAnonBoxes::columnSpanWrapper()) &&
child->GetPrevContinuation(),
"Prev continuation is not set properly?");
}
#endif
}
#ifdef DEBUG_FRAME_DUMP
......@@ -102,6 +95,19 @@ ColumnSetWrapperFrame::AppendFrames(ChildListID aListID,
#endif
nsBlockFrame::AppendFrames(aListID, aFrameList);
#ifdef DEBUG
nsIFrame* firstColumnSet = PrincipalChildList().FirstChild();
for (nsIFrame* child : PrincipalChildList()) {
if (child->IsColumnSpan()) {
AssertColumnSpanWrapperSubtreeIsSane(child);
} else if (child != firstColumnSet) {
// All the other ColumnSets are the continuation of the first ColumnSet.
MOZ_ASSERT(child->IsColumnSetFrame() && child->GetPrevContinuation(),
"ColumnSet's prev-continuation is not set properly?");
}
}
#endif
}
void
......@@ -119,3 +125,37 @@ ColumnSetWrapperFrame::RemoveFrame(ChildListID aListID, nsIFrame* aOldFrame)
MOZ_ASSERT_UNREACHABLE("Unsupported operation!");
nsBlockFrame::RemoveFrame(aListID, aOldFrame);
}
#ifdef DEBUG
/* static */ void
ColumnSetWrapperFrame::AssertColumnSpanWrapperSubtreeIsSane(
const nsIFrame* aFrame) {
MOZ_ASSERT(aFrame->IsColumnSpan(), "aFrame is not column-span?");
if (!aFrame->Style()->IsAnonBox()) {
// aFrame is the primary frame of the element having "column-span: all".
// Traverse no further.
return;
}
MOZ_ASSERT(aFrame->Style()->GetPseudo() == nsCSSAnonBoxes::columnSpanWrapper(),
"aFrame should be ::-moz-column-span-wrapper");
MOZ_ASSERT(!aFrame->HasAnyStateBits(NS_FRAME_OWNS_ANON_BOXES),
"::-moz-column-span-wrapper anonymous blocks cannot own "
"other types of anonymous blocks!");
nsTArray<nsIContent*> anonKids;
nsContentUtils::AppendNativeAnonymousChildren(
aFrame->GetContent(), anonKids, 0);
MOZ_ASSERT(anonKids.IsEmpty(),
"We support only column-span on block and inline frame. They "
"should not create any native anonymous children.");
for (const nsIFrame* child : aFrame->PrincipalChildList()) {
AssertColumnSpanWrapperSubtreeIsSane(child);
}
}
#endif
......@@ -54,6 +54,8 @@ private:
~ColumnSetWrapperFrame() override = default;
#ifdef DEBUG
static void AssertColumnSpanWrapperSubtreeIsSane(const nsIFrame* aFrame);
// True if frame constructor has finished building this frame and all of
// its descendants.
bool mFinishedBuildingColumns = false;
......
......@@ -1231,6 +1231,13 @@ public:
NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(PlaceholderFrameProperty, nsPlaceholderFrame)
// HasColumnSpanSiblings property stores whether the frame has any
// column-span siblings under the same multi-column ancestor. That is, the
// frame's element has column-span descendants without an intervening
// multi-column container element in between them. If the frame having
// this bit set is removed, we need to reframe the multi-column container
NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(HasColumnSpanSiblings, bool)
mozilla::FrameBidiData GetBidiData() const
{
bool exists;
......@@ -3890,9 +3897,14 @@ public:
// Note this only checks computed style, but not testing whether the
// containing block formatting context was established by a multicol.
// Callers need to consider NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR to check
// whether multi-column effects apply or not.
// whether multi-column effects apply or not, or use
// IsColumnSpanInMulticolSubtree().
inline bool IsColumnSpan() const;
// Like IsColumnSpan(), but this also checks whether the frame has a
// multi-column ancestor or not.
inline bool IsColumnSpanInMulticolSubtree() const;
/**
* Returns the vertical-align value to be used for layout, if it is one
* of the enumerated values. If this is an SVG text frame, it returns a value
......
......@@ -97,6 +97,12 @@ nsIFrame::IsColumnSpan() const
return IsBlockOutside() && StyleColumn()->IsColumnSpanStyle();
}
bool
nsIFrame::IsColumnSpanInMulticolSubtree() const
{
return IsColumnSpan() && HasAnyStateBits(NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR);
}
mozilla::StyleDisplay
nsIFrame::GetDisplay() const
{
......
......@@ -975,9 +975,6 @@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit(
nsCSSAnonBoxes::mozBlockInsideInlineWrapper(),
"Unexpected kind of ComputedStyle");
// We don't want to just walk through using GetNextContinuationWithSameStyle
// here, because we want to set updated ComputedStyles on both our
// ib-sibling blocks and inlines.
for (nsIFrame* cont = blockFrame; cont; cont = cont->GetNextContinuation()) {
cont->SetComputedStyle(newContext);
}
......
[multicol-span-all-dynamic-remove-006.html]
prefs: [layout.css.column-span.enabled:true]
[multicol-span-all-restyle-001.html]
prefs: [layout.css.column-span.enabled:true]
[multicol-span-all-restyle-002.html]
prefs: [layout.css.column-span.enabled:true]
[multicol-span-all-restyle-003.html]
prefs: [layout.css.column-span.enabled:true]
[multicol-span-all-restyle-004.html]
prefs: [layout.css.column-span.enabled:true]
<!DOCTYPE html>
<html class="reftest-wait">
<meta charset="utf-8">
<title>CSS Multi-column Layout Test: Remove the parent of a column-spanner</title>
<link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-multicol-1/#column-span">
<link rel="match" href="multicol-span-all-dynamic-remove-001-ref.html">
<meta name="assert" content="This test checks removing the parent of a column-spanner should be rendered correctly.">
<script>
function runTest() {
document.body.offsetHeight;
document.getElementById("block").remove();
document.documentElement.removeAttribute("class");
}
</script>
<style>
#column {
column-count: 3;
column-rule: 6px solid;
width: 400px;
outline: 1px solid black;
}
h3 {
column-span: all;
outline: 1px solid blue;
}
</style>
<body onload="runTest();">
<article id="column">
<div>block1</div>
<div id="block">
<h3>inner spanner</h3>
<div>inner block</div>
</div>
<div>block2</div>
</article>
</body>
</html>
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<title>CSS Multi-column Layout Test Reference: Restyle column-span's parent that is a block</title>
<link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
<style>
#column {
column-count: 1;
width: 400px;
outline: 1px solid black;
}
h3 {
column-span: all;
}
#div {
background-color: yellow;
}
</style>
<body>
<article id="column">
<div id="div">
<div>yellow block1</div>
<h3>spanner (no background-color)</h3>
<div>yellow block2</div>
</div>
</article>
</body>
</html>
<!DOCTYPE html>
<html class="reftest-wait">
<meta charset="utf-8">
<title>CSS Multi-column Layout Test: Restyle column-span's parent that is a block</title>
<link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-multicol-1/#column-span">
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/1072">
<link rel="match" href="multicol-span-all-restyle-001-ref.html">
<meta name="assert" content="This test checks change the style of the spanner's parent should not affect the spanner.">
<script>
function runTest() {
document.body.offsetHeight; // flush layout
var div = document.getElementById("div");
div.style.backgroundColor = "yellow";
document.documentElement.removeAttribute("class");
}
</script>
<style>
#column {
column-count: 1;
width: 400px;
outline: 1px solid black;
}
h3 {
column-span: all;
}
</style>
<body onload="runTest();">
<article id="column">
<div id="div">
<div>yellow block1</div>
<h3>spanner (no background-color)</h3>
<div>yellow block2</div>
</div>
</article>
</body>
</html>
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<title>CSS Multi-column Layout Test Reference: Restyle column-span's parent that is an inline</title>
<link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
<style>
html {
overflow: hidden;
}
#column {
column-count: 1;
}
#span {
position: relative;
left: 200px;
}
h3 {
column-span: all;
}
</style>
<body>
<article id="column">
<span id="span">
All text should be offset 200px, except the spanner
<h3>Spanner</h3>
<div>Some more text</div>
</span>
</article>
</body>
</html>
<!DOCTYPE html>
<html class="reftest-wait">
<meta charset="utf-8">
<title>CSS Multi-column Layout Test: Restyle column-span's parent that is an inline</title>
<link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-multicol-1/#column-span">
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/1072">
<link rel="match" href="multicol-span-all-restyle-002-ref.html">
<meta name="assert" content="This test checks change the style of the spanner's parent should not affect the spanner.">
<script>
function runTest() {
document.body.offsetHeight; // flush layout
var span = document.getElementById("span");
span.offsetWidth;
span.style.left = "200px";
document.documentElement.removeAttribute("class");
}
</script>
<style>
html {
overflow: hidden;
}
#column {
column-count: 1;
}
#span {
position: relative;
left: 100px;
}
h3 {
column-span: all;
}
</style>
<body onload="runTest();">
<article id="column">
<span id="span">
All text should be offset 200px, except the spanner
<h3>Spanner</h3>
<div>Some more text</div>
</span>
</article>
</body>
</html>