- Revision
- 258680
- Author
- [email protected]
- Date
- 2020-03-18 22:02:30 -0700 (Wed, 18 Mar 2020)
Log Message
[Multicolumn] RenderListItem::positionListMarker should not fail when the list marker is inside a spanner.
https://bugs.webkit.org/show_bug.cgi?id=209262
<rdar://problem/58447665>
Reviewed by Simon Fraser.
Source/WebCore:
When the list marker is in a column spanner and as a result it gets moved under the column flow, the
normal "let's find the list item by walking up on the ancestor chain" does not work anymore.
We need to check if this list marker is inside a spanner and climb up on the ancestor chain by
using the spanner placeholder position (see RenderListMarker::parentBox).
This patch also moves the marker's overflow computation from the list item to the marker.
Test: fast/multicol/list-item-marker-inside-column-spanner.html
* rendering/RenderListItem.cpp:
(WebCore::RenderListItem::addOverflowFromChildren):
(WebCore::RenderListItem::positionListMarker): Deleted.
* rendering/RenderListMarker.cpp:
(WebCore::RenderListMarker::parentBox):
(WebCore::RenderListMarker::addOverflowFromListMarker):
(WebCore::RenderListMarker::layout):
* rendering/RenderListMarker.h:
LayoutTests:
* fast/multicol/list-item-marker-inside-column-spanner-expected.txt: Added.
* fast/multicol/list-item-marker-inside-column-spanner.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (258679 => 258680)
--- trunk/LayoutTests/ChangeLog 2020-03-19 04:06:37 UTC (rev 258679)
+++ trunk/LayoutTests/ChangeLog 2020-03-19 05:02:30 UTC (rev 258680)
@@ -1,3 +1,14 @@
+2020-03-18 Zalan Bujtas <[email protected]>
+
+ [Multicolumn] RenderListItem::positionListMarker should not fail when the list marker is inside a spanner.
+ https://bugs.webkit.org/show_bug.cgi?id=209262
+ <rdar://problem/58447665>
+
+ Reviewed by Simon Fraser.
+
+ * fast/multicol/list-item-marker-inside-column-spanner-expected.txt: Added.
+ * fast/multicol/list-item-marker-inside-column-spanner.html: Added.
+
2020-03-18 Simon Fraser <[email protected]>
eventSender.monitorWheelEvents() is very fragile
Added: trunk/LayoutTests/fast/multicol/list-item-marker-inside-column-spanner-expected.txt (0 => 258680)
--- trunk/LayoutTests/fast/multicol/list-item-marker-inside-column-spanner-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/multicol/list-item-marker-inside-column-spanner-expected.txt 2020-03-19 05:02:30 UTC (rev 258680)
@@ -0,0 +1,2 @@
+Pass if no crash or assert when the list marker is inside a column spanner.
+
Added: trunk/LayoutTests/fast/multicol/list-item-marker-inside-column-spanner.html (0 => 258680)
--- trunk/LayoutTests/fast/multicol/list-item-marker-inside-column-spanner.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/list-item-marker-inside-column-spanner.html 2020-03-19 05:02:30 UTC (rev 258680)
@@ -0,0 +1,15 @@
+<span>
+<div style="display: inline-block">
+<ul id=list>
+ <li>
+ <div style="column-span: all;">Pass if no crash or assert when the list marker is inside a column spanner.</div>
+ </li>
+</ul>
+</div>
+</span>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+document.body.offsetHeight;
+list.style.columnCount = 2;
+</script>
Modified: trunk/Source/WebCore/ChangeLog (258679 => 258680)
--- trunk/Source/WebCore/ChangeLog 2020-03-19 04:06:37 UTC (rev 258679)
+++ trunk/Source/WebCore/ChangeLog 2020-03-19 05:02:30 UTC (rev 258680)
@@ -1,3 +1,28 @@
+2020-03-18 Zalan Bujtas <[email protected]>
+
+ [Multicolumn] RenderListItem::positionListMarker should not fail when the list marker is inside a spanner.
+ https://bugs.webkit.org/show_bug.cgi?id=209262
+ <rdar://problem/58447665>
+
+ Reviewed by Simon Fraser.
+
+ When the list marker is in a column spanner and as a result it gets moved under the column flow, the
+ normal "let's find the list item by walking up on the ancestor chain" does not work anymore.
+ We need to check if this list marker is inside a spanner and climb up on the ancestor chain by
+ using the spanner placeholder position (see RenderListMarker::parentBox).
+ This patch also moves the marker's overflow computation from the list item to the marker.
+
+ Test: fast/multicol/list-item-marker-inside-column-spanner.html
+
+ * rendering/RenderListItem.cpp:
+ (WebCore::RenderListItem::addOverflowFromChildren):
+ (WebCore::RenderListItem::positionListMarker): Deleted.
+ * rendering/RenderListMarker.cpp:
+ (WebCore::RenderListMarker::parentBox):
+ (WebCore::RenderListMarker::addOverflowFromListMarker):
+ (WebCore::RenderListMarker::layout):
+ * rendering/RenderListMarker.h:
+
2020-03-18 Simon Fraser <[email protected]>
eventSender.monitorWheelEvents() is very fragile
Modified: trunk/Source/WebCore/rendering/RenderListItem.cpp (258679 => 258680)
--- trunk/Source/WebCore/rendering/RenderListItem.cpp 2020-03-19 04:06:37 UTC (rev 258679)
+++ trunk/Source/WebCore/rendering/RenderListItem.cpp 2020-03-19 05:02:30 UTC (rev 258680)
@@ -248,7 +248,8 @@
void RenderListItem::addOverflowFromChildren()
{
- positionListMarker();
+ if (m_marker)
+ m_marker->addOverflowFromListMarker();
RenderBlockFlow::addOverflowFromChildren();
}
@@ -261,102 +262,6 @@
RenderBlockFlow::computePreferredLogicalWidths();
}
-void RenderListItem::positionListMarker()
-{
- if (!m_marker || !m_marker->parent() || !m_marker->parent()->isBox())
- return;
-
- if (m_marker->isInside() || !m_marker->inlineBoxWrapper())
- return;
-
- LayoutUnit markerOldLogicalLeft = m_marker->logicalLeft();
- LayoutUnit blockOffset;
- LayoutUnit lineOffset;
- for (auto* ancestor = m_marker->parentBox(); ancestor && ancestor != this; ancestor = ancestor->parentBox()) {
- blockOffset += ancestor->logicalTop();
- lineOffset += ancestor->logicalLeft();
- }
-
- bool adjustOverflow = false;
- LayoutUnit markerLogicalLeft;
- bool hitSelfPaintingLayer = false;
-
- const RootInlineBox& rootBox = m_marker->inlineBoxWrapper()->root();
- LayoutUnit lineTop = rootBox.lineTop();
- LayoutUnit lineBottom = rootBox.lineBottom();
-
- // FIXME: Need to account for relative positioning in the layout overflow.
- if (style().isLeftToRightDirection()) {
- markerLogicalLeft = m_marker->lineOffsetForListItem() - lineOffset - paddingStart() - borderStart() + m_marker->marginStart();
- m_marker->inlineBoxWrapper()->adjustLineDirectionPosition(markerLogicalLeft - markerOldLogicalLeft);
- for (InlineFlowBox* box = m_marker->inlineBoxWrapper()->parent(); box; box = box->parent()) {
- LayoutRect newLogicalVisualOverflowRect = box->logicalVisualOverflowRect(lineTop, lineBottom);
- LayoutRect newLogicalLayoutOverflowRect = box->logicalLayoutOverflowRect(lineTop, lineBottom);
- if (markerLogicalLeft < newLogicalVisualOverflowRect.x() && !hitSelfPaintingLayer) {
- newLogicalVisualOverflowRect.setWidth(newLogicalVisualOverflowRect.maxX() - markerLogicalLeft);
- newLogicalVisualOverflowRect.setX(markerLogicalLeft);
- if (box == &rootBox)
- adjustOverflow = true;
- }
- if (markerLogicalLeft < newLogicalLayoutOverflowRect.x()) {
- newLogicalLayoutOverflowRect.setWidth(newLogicalLayoutOverflowRect.maxX() - markerLogicalLeft);
- newLogicalLayoutOverflowRect.setX(markerLogicalLeft);
- if (box == &rootBox)
- adjustOverflow = true;
- }
- box->setOverflowFromLogicalRects(newLogicalLayoutOverflowRect, newLogicalVisualOverflowRect, lineTop, lineBottom);
- if (box->renderer().hasSelfPaintingLayer())
- hitSelfPaintingLayer = true;
- }
- } else {
- markerLogicalLeft = m_marker->lineOffsetForListItem() - lineOffset + paddingStart() + borderStart() + m_marker->marginEnd();
- m_marker->inlineBoxWrapper()->adjustLineDirectionPosition(markerLogicalLeft - markerOldLogicalLeft);
- for (InlineFlowBox* box = m_marker->inlineBoxWrapper()->parent(); box; box = box->parent()) {
- LayoutRect newLogicalVisualOverflowRect = box->logicalVisualOverflowRect(lineTop, lineBottom);
- LayoutRect newLogicalLayoutOverflowRect = box->logicalLayoutOverflowRect(lineTop, lineBottom);
- if (markerLogicalLeft + m_marker->logicalWidth() > newLogicalVisualOverflowRect.maxX() && !hitSelfPaintingLayer) {
- newLogicalVisualOverflowRect.setWidth(markerLogicalLeft + m_marker->logicalWidth() - newLogicalVisualOverflowRect.x());
- if (box == &rootBox)
- adjustOverflow = true;
- }
- if (markerLogicalLeft + m_marker->logicalWidth() > newLogicalLayoutOverflowRect.maxX()) {
- newLogicalLayoutOverflowRect.setWidth(markerLogicalLeft + m_marker->logicalWidth() - newLogicalLayoutOverflowRect.x());
- if (box == &rootBox)
- adjustOverflow = true;
- }
- box->setOverflowFromLogicalRects(newLogicalLayoutOverflowRect, newLogicalVisualOverflowRect, lineTop, lineBottom);
-
- if (box->renderer().hasSelfPaintingLayer())
- hitSelfPaintingLayer = true;
- }
- }
-
- if (adjustOverflow) {
- LayoutRect markerRect(markerLogicalLeft + lineOffset, blockOffset, m_marker->width(), m_marker->height());
- if (!style().isHorizontalWritingMode())
- markerRect = markerRect.transposedRect();
- RenderBox* markerAncestor = m_marker.get();
- bool propagateVisualOverflow = true;
- bool propagateLayoutOverflow = true;
- do {
- markerAncestor = markerAncestor->parentBox();
- if (markerAncestor->hasOverflowClip())
- propagateVisualOverflow = false;
- if (is<RenderBlock>(*markerAncestor)) {
- if (propagateVisualOverflow)
- downcast<RenderBlock>(*markerAncestor).addVisualOverflow(markerRect);
- if (propagateLayoutOverflow)
- downcast<RenderBlock>(*markerAncestor).addLayoutOverflow(markerRect);
- }
- if (markerAncestor->hasOverflowClip())
- propagateLayoutOverflow = false;
- if (markerAncestor->hasSelfPaintingLayer())
- propagateVisualOverflow = false;
- markerRect.moveBy(-markerAncestor->location());
- } while (markerAncestor != this && propagateVisualOverflow && propagateLayoutOverflow);
- }
-}
-
void RenderListItem::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
{
if (!logicalHeight() && hasOverflowClip())
Modified: trunk/Source/WebCore/rendering/RenderListMarker.cpp (258679 => 258680)
--- trunk/Source/WebCore/rendering/RenderListMarker.cpp 2020-03-19 04:06:37 UTC (rev 258679)
+++ trunk/Source/WebCore/rendering/RenderListMarker.cpp 2020-03-19 05:02:30 UTC (rev 258680)
@@ -31,6 +31,8 @@
#include "InlineElementBox.h"
#include "RenderLayer.h"
#include "RenderListItem.h"
+#include "RenderMultiColumnFlow.h"
+#include "RenderMultiColumnSpannerPlaceholder.h"
#include "RenderView.h"
#include <wtf/IsoMallocInlines.h>
#include <wtf/StackStats.h>
@@ -1319,6 +1321,114 @@
}
}
+RenderBox* RenderListMarker::parentBox(RenderBox& box)
+{
+ ASSERT(m_listItem);
+ auto* fragmentedFlow = m_listItem->enclosingFragmentedFlow();
+ if (!is<RenderMultiColumnFlow>(fragmentedFlow))
+ return box.parentBox();
+ auto* placeholder = downcast<RenderMultiColumnFlow>(*fragmentedFlow).findColumnSpannerPlaceholder(&box);
+ if (!placeholder)
+ return box.parentBox();
+ return placeholder->parentBox();
+};
+
+void RenderListMarker::addOverflowFromListMarker()
+{
+ ASSERT(m_listItem);
+ if (!parent() || !parent()->isBox())
+ return;
+
+ if (isInside() || !inlineBoxWrapper())
+ return;
+
+ LayoutUnit markerOldLogicalLeft = logicalLeft();
+ LayoutUnit blockOffset;
+ LayoutUnit lineOffset;
+ for (auto* ancestor = parentBox(*this); ancestor && ancestor != m_listItem.get(); ancestor = parentBox(*ancestor)) {
+ blockOffset += ancestor->logicalTop();
+ lineOffset += ancestor->logicalLeft();
+ }
+
+ bool adjustOverflow = false;
+ LayoutUnit markerLogicalLeft;
+ bool hitSelfPaintingLayer = false;
+
+ const RootInlineBox& rootBox = inlineBoxWrapper()->root();
+ LayoutUnit lineTop = rootBox.lineTop();
+ LayoutUnit lineBottom = rootBox.lineBottom();
+
+ // FIXME: Need to account for relative positioning in the layout overflow.
+ if (m_listItem->style().isLeftToRightDirection()) {
+ markerLogicalLeft = lineOffsetForListItem() - lineOffset - m_listItem->paddingStart() - m_listItem->borderStart() + marginStart();
+ inlineBoxWrapper()->adjustLineDirectionPosition(markerLogicalLeft - markerOldLogicalLeft);
+ for (auto* box = inlineBoxWrapper()->parent(); box; box = box->parent()) {
+ auto newLogicalVisualOverflowRect = box->logicalVisualOverflowRect(lineTop, lineBottom);
+ auto newLogicalLayoutOverflowRect = box->logicalLayoutOverflowRect(lineTop, lineBottom);
+ if (markerLogicalLeft < newLogicalVisualOverflowRect.x() && !hitSelfPaintingLayer) {
+ newLogicalVisualOverflowRect.setWidth(newLogicalVisualOverflowRect.maxX() - markerLogicalLeft);
+ newLogicalVisualOverflowRect.setX(markerLogicalLeft);
+ if (box == &rootBox)
+ adjustOverflow = true;
+ }
+ if (markerLogicalLeft < newLogicalLayoutOverflowRect.x()) {
+ newLogicalLayoutOverflowRect.setWidth(newLogicalLayoutOverflowRect.maxX() - markerLogicalLeft);
+ newLogicalLayoutOverflowRect.setX(markerLogicalLeft);
+ if (box == &rootBox)
+ adjustOverflow = true;
+ }
+ box->setOverflowFromLogicalRects(newLogicalLayoutOverflowRect, newLogicalVisualOverflowRect, lineTop, lineBottom);
+ if (box->renderer().hasSelfPaintingLayer())
+ hitSelfPaintingLayer = true;
+ }
+ } else {
+ markerLogicalLeft = lineOffsetForListItem() - lineOffset + m_listItem->paddingStart() + m_listItem->borderStart() + marginEnd();
+ inlineBoxWrapper()->adjustLineDirectionPosition(markerLogicalLeft - markerOldLogicalLeft);
+ for (auto* box = inlineBoxWrapper()->parent(); box; box = box->parent()) {
+ auto newLogicalVisualOverflowRect = box->logicalVisualOverflowRect(lineTop, lineBottom);
+ auto newLogicalLayoutOverflowRect = box->logicalLayoutOverflowRect(lineTop, lineBottom);
+ if (markerLogicalLeft + logicalWidth() > newLogicalVisualOverflowRect.maxX() && !hitSelfPaintingLayer) {
+ newLogicalVisualOverflowRect.setWidth(markerLogicalLeft + logicalWidth() - newLogicalVisualOverflowRect.x());
+ if (box == &rootBox)
+ adjustOverflow = true;
+ }
+ if (markerLogicalLeft + logicalWidth() > newLogicalLayoutOverflowRect.maxX()) {
+ newLogicalLayoutOverflowRect.setWidth(markerLogicalLeft + logicalWidth() - newLogicalLayoutOverflowRect.x());
+ if (box == &rootBox)
+ adjustOverflow = true;
+ }
+ box->setOverflowFromLogicalRects(newLogicalLayoutOverflowRect, newLogicalVisualOverflowRect, lineTop, lineBottom);
+ if (box->renderer().hasSelfPaintingLayer())
+ hitSelfPaintingLayer = true;
+ }
+ }
+
+ if (adjustOverflow) {
+ LayoutRect markerRect(markerLogicalLeft + lineOffset, blockOffset, width(), height());
+ if (!m_listItem->style().isHorizontalWritingMode())
+ markerRect = markerRect.transposedRect();
+ RenderBox* markerAncestor = this;
+ bool propagateVisualOverflow = true;
+ bool propagateLayoutOverflow = true;
+ do {
+ markerAncestor = parentBox(*markerAncestor);
+ if (markerAncestor->hasOverflowClip())
+ propagateVisualOverflow = false;
+ if (is<RenderBlock>(*markerAncestor)) {
+ if (propagateVisualOverflow)
+ downcast<RenderBlock>(*markerAncestor).addVisualOverflow(markerRect);
+ if (propagateLayoutOverflow)
+ downcast<RenderBlock>(*markerAncestor).addLayoutOverflow(markerRect);
+ }
+ if (markerAncestor->hasOverflowClip())
+ propagateLayoutOverflow = false;
+ if (markerAncestor->hasSelfPaintingLayer())
+ propagateVisualOverflow = false;
+ markerRect.moveBy(-markerAncestor->location());
+ } while (markerAncestor != m_listItem.get() && propagateVisualOverflow && propagateLayoutOverflow);
+ }
+}
+
void RenderListMarker::layout()
{
StackStats::LayoutCheckPoint layoutCheckPoint;
@@ -1325,7 +1435,7 @@
ASSERT(needsLayout());
LayoutUnit blockOffset;
- for (auto* ancestor = parentBox(); ancestor && ancestor != m_listItem.get(); ancestor = ancestor->parentBox())
+ for (auto* ancestor = parentBox(*this); ancestor && ancestor != m_listItem.get(); ancestor = parentBox(*ancestor))
blockOffset += ancestor->logicalTop();
if (style().isLeftToRightDirection())
m_lineOffsetForListItem = m_listItem->logicalLeftOffsetForLine(blockOffset, DoNotIndentText, 0_lu);
Modified: trunk/Source/WebCore/rendering/RenderListMarker.h (258679 => 258680)
--- trunk/Source/WebCore/rendering/RenderListMarker.h 2020-03-19 04:06:37 UTC (rev 258679)
+++ trunk/Source/WebCore/rendering/RenderListMarker.h 2020-03-19 05:02:30 UTC (rev 258680)
@@ -47,6 +47,8 @@
void updateMarginsAndContent();
+ void addOverflowFromListMarker();
+
private:
void willBeDestroyed() override;
@@ -81,6 +83,8 @@
void styleDidChange(StyleDifference, const RenderStyle* oldStyle) override;
+ RenderBox* parentBox(RenderBox&);
+
FloatRect getRelativeMarkerRect();
LayoutRect localSelectionRect();
