Out-of-flow <br> should not trigger a line break

https://bugs.webkit.org/show_bug.cgi?id=259631 Reviewed by Antti Koivisto. This is just to make sure we don't include out-of-flow line break in inflow inline layout producing forced line break at such boxes. However this patch does not address the incorrect offsetTop/left/client rect etc values. (In order to support a full-fledged out-of-flow line break, we would need to inherit RenderLineBreak from RenderBox which is quite a memory waste considering RenderLineBreak is in-flow most of the time -also, it would require some involved changes as currently some code assumes that RenderLineBreak is never a RenderBox.) * LayoutTests/fast/inline/out-of-flow-forced-line-break-expected.html: Added. * LayoutTests/fast/inline/out-of-flow-forced-line-break.html: Added. * Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp: (WebCore::Layout::InlineItemsBuilder::collectInlineItems): * Source/WebCore/layout/integration/LayoutIntegrationBoxTree.cpp: (WebCore::LayoutIntegration::BoxTree::adjustStyleIfNeeded): * Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp: (WebCore::LayoutIntegration::LineLayout::updateLineBreakBoxDimensions): (WebCore::LayoutIntegration::LineLayout::updateRenderTreePositions): Canonical link: https://commits.webkit.org/266475@main
WebKit · Aug 1, 2023 · f71ea9e · f71ea9e
1 parent c22c66c
commit f71ea9e
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 9 deletions.
diff --git a/LayoutTests/fast/inline/out-of-flow-forced-line-break-expected.html b/LayoutTests/fast/inline/out-of-flow-forced-line-break-expected.html
@@ -0,0 +1,7 @@
+<style>
+div {
+  font-family: monospace;
+}
+</style>
+<div>PASS if no line break.</div>
+<div>PASS if no line break.</div>
diff --git a/LayoutTests/fast/inline/out-of-flow-forced-line-break.html b/LayoutTests/fast/inline/out-of-flow-forced-line-break.html
@@ -0,0 +1,7 @@
+<style>
+div {
+  font-family: monospace;
+}
+</style>
+<div>PASS if<br style="position: absolute;"> no line break.</div>
+<div>PASS if<br style="position: fixed;"> no line break.</div>
diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp
@@ -232,7 +232,11 @@ void InlineItemsBuilder::collectInlineItems(InlineItems& inlineItems, Formatting
 
         while (!layoutQueue.isEmpty()) {
             auto layoutBox = layoutQueue.takeLast();
-            if (layoutBox->isInlineTextBox()) {
+            if (layoutBox->isOutOfFlowPositioned()) {
+                // Let's not construct InlineItems for out-of-flow content as they don't participate in the inline layout.
+                // However to be able to static positioning them, we need to compute their approximate positions.
+                outOfFlowBoxes.append(layoutBox);
+            } else if (layoutBox->isInlineTextBox()) {
                 auto& inlineTextBox = downcast<InlineTextBox>(layoutBox);
                 handleTextContent(inlineTextBox, inlineItems, partialContentOffset(inlineTextBox));
             } else if (layoutBox->isAtomicInlineLevelBox() || layoutBox->isLineBreakBox())
@@ -241,11 +245,7 @@ void InlineItemsBuilder::collectInlineItems(InlineItems& inlineItems, Formatting
                 handleInlineBoxEnd(layoutBox, inlineItems);
             else if (layoutBox->isFloatingPositioned())
                 inlineItems.append({ layoutBox, InlineItem::Type::Float });
-            else if (layoutBox->isOutOfFlowPositioned()) {
-                // Let's not construct InlineItems for out-of-flow content as they don't participate in the inline layout.
-                // However to be able to static positioning them, we need to compute their approximate positions.
-                outOfFlowBoxes.append(layoutBox);
-            } else
+            else
                 ASSERT_NOT_REACHED();
 
             if (auto* nextSibling = layoutBox->nextSibling()) {

diff --git a/Source/WebCore/layout/integration/LayoutIntegrationBoxTree.cpp b/Source/WebCore/layout/integration/LayoutIntegrationBoxTree.cpp
@@ -162,9 +162,11 @@ void BoxTree::adjustStyleIfNeeded(const RenderElement& renderer, RenderStyle& st
             return;
         }
         if (renderer.isLineBreak()) {
-            styleToAdjust.setDisplay(DisplayType::Inline);
+            if (!styleToAdjust.hasOutOfFlowPosition()) {
+                // Force in-flow display value to inline (see webkit.org/b/223151).
+                styleToAdjust.setDisplay(DisplayType::Inline);
+            }
             styleToAdjust.setFloating(Float::None);
-            styleToAdjust.setPosition(PositionType::Static);
             // Clear property should only apply on block elements, however,
             // it appears that browsers seem to ignore it on <br> inline elements.
             // https://drafts.csswg.org/css2/#propdef-clear

diff --git a/Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp b/Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp
@@ -453,6 +453,8 @@ void LineLayout::updateLineBreakBoxDimensions(const RenderLineBreak& lineBreakBo
     boxGeometry.setPadding({ });
     boxGeometry.setContentBoxWidth({ });
     boxGeometry.setVerticalMargin({ });
+    if (lineBreakBox.style().hasOutOfFlowPosition())
+        boxGeometry.setContentBoxHeight({ });
 }
 
 void LineLayout::updateInlineBoxDimensions(const RenderInline& renderInline)
@@ -654,7 +656,8 @@ void LineLayout::updateRenderTreePositions(const Vector<LineAdjustment>& lineAdj
         auto& layoutBox = *renderObject->layoutBox();
         if (!layoutBox.isFloatingPositioned() && !layoutBox.isOutOfFlowPositioned())
             continue;
-
+        if (layoutBox.isLineBreakBox())
+            continue;
         auto& renderer = downcast<RenderBox>(m_boxTree.rendererForLayoutBox(layoutBox));
         auto& logicalGeometry = m_inlineFormattingState.boxGeometry(layoutBox);