[css-multicol] Restore placeholders on multicolumn children position …

…changes. https://bugs.webkit.org/show_bug.cgi?id=218501 Reviewed by Zalan Bujtas. Whenever there is a placeholder in a multicolumn flow ("column-span: all") it must be ensured that the spanner associated to that placeholder is properly restored as a child of the multicolumn flow when the element with "column-span:all" is moved out of the multicolumn flow (for example by removing it, by making it out-of-flow or because an ancestor becomes another multicolumn flow). This was smoothly handled by the current code for the in-flow -> out-of-flow transition but not the other way around, i.e in the case of making the placeholder (or an ancestor) an absolutely positioned element. * rendering/updating/RenderTreeBuilder.cpp: (WebCore::RenderTreeBuilder::normalizeTreeAfterStyleChange): restore column spanners then the element becomes out-of-flow as it's removed from the multicolumn flow. * rendering/updating/RenderTreeBuilderMultiColumn.cpp: (WebCore::RenderTreeBuilder::MultiColumn::createFragmentedFlow): Call the newly created method restoreColumnSpannersForContainer(). (WebCore::RenderTreeBuilder::MultiColumn::restoreColumnSpannersForContainer): Refactored from createFragmentedFlow() as it's now used by normalizeTreeAfterStyleChange(). * rendering/updating/RenderTreeBuilderMultiColumn.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@271357 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima · Jan 11, 2021 · fdefed4 · fdefed4
1 parent 55ed007
commit fdefed4
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 26 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2021-01-07  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-multicol] Restore placeholders on multicolumn children position changes.
+        https://bugs.webkit.org/show_bug.cgi?id=218501
+
+        Reviewed by Zalan Bujtas.
+
+        Whenever there is a placeholder in a multicolumn flow ("column-span: all") it must be ensured that the spanner
+        associated to that placeholder is properly restored as a child of the multicolumn flow when the element with
+        "column-span:all" is moved out of the multicolumn flow (for example by removing it, by making it out-of-flow
+        or because an ancestor becomes another multicolumn flow).
+
+        This was smoothly handled by the current code for the in-flow -> out-of-flow transition but not the other way
+        around, i.e in the case of making the placeholder (or an ancestor) an absolutely positioned element.
+
+        * rendering/updating/RenderTreeBuilder.cpp:
+        (WebCore::RenderTreeBuilder::normalizeTreeAfterStyleChange): restore column spanners then the element becomes
+        out-of-flow as it's removed from the multicolumn flow.
+        * rendering/updating/RenderTreeBuilderMultiColumn.cpp:
+        (WebCore::RenderTreeBuilder::MultiColumn::createFragmentedFlow): Call the newly created method
+        restoreColumnSpannersForContainer().
+        (WebCore::RenderTreeBuilder::MultiColumn::restoreColumnSpannersForContainer): Refactored from createFragmentedFlow()
+        as it's now used by normalizeTreeAfterStyleChange().
+        * rendering/updating/RenderTreeBuilderMultiColumn.h:
+
 2020-12-04  Sergio Villar Senin  <svillar@igalia.com>
 
         [css-multicol] Update fragment flow state on element insertion when element position changes

diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp
@@ -598,9 +598,18 @@ void RenderTreeBuilder::normalizeTreeAfterStyleChange(RenderElement& renderer, R
 
     // Out of flow children of RenderMultiColumnFlow are not really part of the multicolumn flow. We need to ensure that changes in positioning like this
     // trigger insertions into the multicolumn flow.
-    if (auto* enclosingFragmentedFlow = parent.enclosingFragmentedFlow(); is<RenderMultiColumnFlow>(enclosingFragmentedFlow) && wasOutOfFlowPositioned && !isOutOfFlowPositioned) {
-        multiColumnBuilder().multiColumnDescendantInserted(downcast<RenderMultiColumnFlow>(*enclosingFragmentedFlow), renderer);
-        renderer.initializeFragmentedFlowStateOnInsertion();
+    if (auto* enclosingFragmentedFlow = parent.enclosingFragmentedFlow(); is<RenderMultiColumnFlow>(enclosingFragmentedFlow)) {
+        auto movingIntoMulticolumn = wasOutOfFlowPositioned && !isOutOfFlowPositioned;
+        if (movingIntoMulticolumn) {
+            multiColumnBuilder().multiColumnDescendantInserted(downcast<RenderMultiColumnFlow>(*enclosingFragmentedFlow), renderer);
+            renderer.initializeFragmentedFlowStateOnInsertion();
+            return;
+        }
+        auto movingOutOfMulticolumn = !wasOutOfFlowPositioned && isOutOfFlowPositioned;
+        if (movingOutOfMulticolumn) {
+            multiColumnBuilder().restoreColumnSpannersForContainer(renderer, downcast<RenderMultiColumnFlow>(*enclosingFragmentedFlow));
+            return;
+        }
     }
 }
 

diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp
@@ -153,29 +153,8 @@ void RenderTreeBuilder::MultiColumn::createFragmentedFlow(RenderBlockFlow& flow)
     flow.deleteLines();
     // If this soon-to-be multicolumn flow is already part of a multicolumn context, we need to move back the descendant spanners
     // to their original position before moving subtrees around.
-    auto* enclosingflow = flow.enclosingFragmentedFlow();
-    if (is<RenderMultiColumnFlow>(enclosingflow)) {
-        auto& spanners = downcast<RenderMultiColumnFlow>(enclosingflow)->spannerMap();
-        Vector<RenderMultiColumnSpannerPlaceholder*> placeholdersToDelete;
-        for (auto& spannerAndPlaceholder : spanners) {
-            auto& placeholder = *spannerAndPlaceholder.value;
-            if (!placeholder.isDescendantOf(&flow))
-                continue;
-            placeholdersToDelete.append(&placeholder);
-        }
-        for (auto* placeholder : placeholdersToDelete) {
-            auto* spanner = placeholder->spanner();
-            if (!spanner) {
-                ASSERT_NOT_REACHED();
-                continue;
-            }
-            // Move the spanner back to its original position.
-            auto& spannerOriginalParent = *placeholder->parent();
-            // Detaching the spanner takes care of removing the placeholder (and merges the RenderMultiColumnSets).
-            auto spannerToReInsert = m_builder.detach(*spanner->parent(), *spanner);
-            m_builder.attach(spannerOriginalParent, WTFMove(spannerToReInsert));
-        }
-    }
+    if (auto* enclosingflow = flow.enclosingFragmentedFlow(); is<RenderMultiColumnFlow>(enclosingflow))
+        restoreColumnSpannersForContainer(flow, downcast<RenderMultiColumnFlow>(*enclosingflow));
 
     auto newFragmentedFlow = WebCore::createRenderer<RenderMultiColumnFlow>(flow.document(), RenderStyle::createAnonymousStyleWithDisplay(flow.style(), DisplayType::Block));
     newFragmentedFlow->initializeStyle();
@@ -195,6 +174,30 @@ void RenderTreeBuilder::MultiColumn::createFragmentedFlow(RenderBlockFlow& flow)
     flow.setMultiColumnFlow(fragmentedFlow);
 }
 
+void RenderTreeBuilder::MultiColumn::restoreColumnSpannersForContainer(const RenderElement& container, RenderMultiColumnFlow& multiColumnFlow)
+{
+    auto& spanners = multiColumnFlow.spannerMap();
+    Vector<RenderMultiColumnSpannerPlaceholder*> placeholdersToRestore;
+    for (auto& spannerAndPlaceholder : spanners) {
+        auto& placeholder = *spannerAndPlaceholder.value;
+        if (!placeholder.isDescendantOf(&container))
+            continue;
+        placeholdersToRestore.append(&placeholder);
+    }
+    for (auto* placeholder : placeholdersToRestore) {
+        auto* spanner = placeholder->spanner();
+        if (!spanner) {
+            ASSERT_NOT_REACHED();
+            continue;
+        }
+        // Move the spanner back to its original position.
+        auto& spannerOriginalParent = *placeholder->parent();
+        // Detaching the spanner takes care of removing the placeholder (and merges the RenderMultiColumnSets).
+        auto spannerToReInsert = m_builder.detach(*spanner->parent(), *spanner);
+        m_builder.attach(spannerOriginalParent, WTFMove(spannerToReInsert));
+    }
+}
+
 void RenderTreeBuilder::MultiColumn::destroyFragmentedFlow(RenderBlockFlow& flow)
 {
     auto& multiColumnFlow = *flow.multiColumnFlow();

diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.h b/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.h
@@ -42,6 +42,7 @@ class RenderTreeBuilder::MultiColumn {
     // sets. If |child| is such a renderer, resolve it to the placeholder that lives at the original
     // location in the tree.
     RenderObject* resolveMovedChild(RenderFragmentedFlow& enclosingFragmentedFlow, RenderObject* beforeChild);
+    void restoreColumnSpannersForContainer(const RenderElement& container, RenderMultiColumnFlow&);
     void multiColumnDescendantInserted(RenderMultiColumnFlow&, RenderObject& newDescendant);
     void multiColumnRelativeWillBeRemoved(RenderMultiColumnFlow&, RenderObject& relative);
     static RenderObject* adjustBeforeChildForMultiColumnSpannerIfNeeded(RenderObject& beforeChild);