Diff
Modified: trunk/Source/WebCore/ChangeLog (273222 => 273223)
--- trunk/Source/WebCore/ChangeLog 2021-02-21 21:08:03 UTC (rev 273222)
+++ trunk/Source/WebCore/ChangeLog 2021-02-21 21:38:41 UTC (rev 273223)
@@ -1,3 +1,39 @@
+2021-02-21 Zalan Bujtas <[email protected]>
+
+ [LFC][IFC] Hittest should be using the border box (and not the margin box)
+ https://bugs.webkit.org/show_bug.cgi?id=222249
+
+ Reviewed by Antti Koivisto.
+
+ This patch fixes the cases when the inline box (e.g. <span>) has horizontal
+ margins and we find those inline boxes instead of their containing blocks.
+ e.g.
+ <div><span style="margin-left: 100px;">border box starts at 100px</span></div>
+ document.elementFromPoint(50, 10) should not find the <span>.
+
+ Let's simplify LineBox interface so that it returns the border box. Clients
+ are mainly interested in the border box.
+ (LineBox is mostly there to support vertical alignment and the vertical
+ alignment uses the margin box height. The inline level boxes on the line now have
+ this seemingly odd combination of border box width and margin box height, but
+ that's required to keep the alignment logic clean and simple.)
+
+ * layout/inlineformatting/InlineFormattingContext.cpp:
+ (WebCore::Layout::InlineFormattingContext::computeGeometryForLineContent):
+ * layout/inlineformatting/InlineFormattingContextGeometry.cpp:
+ (WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
+ * layout/inlineformatting/InlineLineBox.cpp:
+ (WebCore::Layout::LineBox::logicalBorderBoxForAtomicInlineLevelBox const):
+ (WebCore::Layout::LineBox::logicalBorderBoxForInlineBox const):
+ (WebCore::Layout::LineBox::logicalMarginRectForAtomicInlineLevelBox const): Deleted.
+ (WebCore::Layout::LineBox::logicalRectForInlineBox const): Deleted.
+ * layout/inlineformatting/InlineLineBox.h:
+ * layout/integration/LayoutIntegrationInlineContentBuilder.cpp:
+ (WebCore::LayoutIntegration::InlineContentBuilder::createDisplayLineRuns const):
+ (WebCore::LayoutIntegration::InlineContentBuilder::createDisplayNonRootInlineBoxes const):
+ * layout/layouttree/LayoutTreeBuilder.cpp:
+ (WebCore::Layout::showInlineTreeAndRuns):
+
2021-02-21 Noam Rosenthal <[email protected]>
[Paint Timing] Return early from contentful paint check when no contentful pixels/characters at all
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp (273222 => 273223)
--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp 2021-02-21 21:08:03 UTC (rev 273222)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp 2021-02-21 21:38:41 UTC (rev 273223)
@@ -466,7 +466,7 @@
}
for (auto* layoutBox : layoutBoxList) {
auto& boxGeometry = formattingState.boxGeometry(*layoutBox);
- auto inlineBoxLogicalHeight = LayoutUnit::fromFloatCeil(lineBox.logicalRectForInlineBox(*layoutBox, boxGeometry).height());
+ auto inlineBoxLogicalHeight = LayoutUnit::fromFloatCeil(lineBox.logicalBorderBoxForInlineBox(*layoutBox, boxGeometry).height());
boxGeometry.setContentBoxHeight(inlineBoxLogicalHeight);
boxGeometry.setContentBoxWidth({ });
boxGeometry.setLogicalTopLeft(toLayoutPoint(lineBoxLogicalRect.topLeft()));
@@ -507,12 +507,11 @@
}
if (lineRun.isBox()) {
ASSERT(layoutBox.isAtomicInlineLevelBox());
- auto logicalMarginRect = lineBox.logicalMarginRectForAtomicInlineLevelBox(layoutBox);
- formattingState.addLineRun({ lineIndex, layoutBox, logicalMarginRect, lineRun.expansion(), { } });
+ auto& boxGeometry = formattingState.boxGeometry(layoutBox);
+ auto logicalBorderBox = lineBox.logicalBorderBoxForAtomicInlineLevelBox(layoutBox, boxGeometry);
+ formattingState.addLineRun({ lineIndex, layoutBox, logicalBorderBox, lineRun.expansion(), { } });
- auto borderBoxLogicalTopLeft = logicalMarginRect.topLeft();
- auto& boxGeometry = formattingState.boxGeometry(layoutBox);
- borderBoxLogicalTopLeft.move(std::max(0_lu, boxGeometry.marginStart()), std::max(0_lu, boxGeometry.marginBefore()));
+ auto borderBoxLogicalTopLeft = logicalBorderBox.topLeft();
// Note that inline boxes are relative to the line and their top position can be negative.
borderBoxLogicalTopLeft.moveBy(lineBoxLogicalRect.topLeft());
if (layoutBox.isInFlowPositioned())
@@ -528,7 +527,7 @@
}
if (lineRun.isInlineBoxStart()) {
auto& boxGeometry = formattingState.boxGeometry(layoutBox);
- auto inlineBoxLogicalRect = lineBox.logicalRectForInlineBox(layoutBox, boxGeometry);
+ auto inlineBoxLogicalRect = lineBox.logicalBorderBoxForInlineBox(layoutBox, boxGeometry);
formattingState.addLineRun({ lineIndex, layoutBox, inlineBoxLogicalRect, lineRun.expansion(), { } });
inlineBoxStartSet.add(&layoutBox);
enclosingTopAndBottom.top = std::min(enclosingTopAndBottom.top, inlineBoxLogicalRect.top());
@@ -536,7 +535,7 @@
}
if (lineRun.isInlineBoxEnd()) {
inlineBoxEndSet.add(&layoutBox);
- auto inlineBoxLogicalRect = lineBox.logicalRectForInlineBox(layoutBox, formattingState.boxGeometry(layoutBox));
+ auto inlineBoxLogicalRect = lineBox.logicalBorderBoxForInlineBox(layoutBox, formattingState.boxGeometry(layoutBox));
enclosingTopAndBottom.bottom = std::max(enclosingTopAndBottom.bottom, inlineBoxLogicalRect.bottom());
continue;
}
@@ -557,30 +556,20 @@
auto& layoutBox = inlineLevelBox->layoutBox();
auto& boxGeometry = formattingState.boxGeometry(layoutBox);
// Inline boxes may or may not be wrapped and have runs on multiple lines (e.g. <span>first line<br>second line<br>third line</span>)
- auto inlineBoxMarginRect = lineBox.logicalRectForInlineBox(layoutBox, boxGeometry);
- auto inlineBoxSize = LayoutSize { LayoutUnit::fromFloatCeil(inlineBoxMarginRect.width()), LayoutUnit::fromFloatCeil(inlineBoxMarginRect.height()) };
- auto logicalRect = Rect { LayoutPoint { inlineBoxMarginRect.topLeft() }, inlineBoxSize };
+ auto inlineBoxBorderBox = lineBox.logicalBorderBoxForInlineBox(layoutBox, boxGeometry);
+ auto inlineBoxSize = LayoutSize { LayoutUnit::fromFloatCeil(inlineBoxBorderBox.width()), LayoutUnit::fromFloatCeil(inlineBoxBorderBox.height()) };
+ auto logicalRect = Rect { LayoutPoint { inlineBoxBorderBox.topLeft() }, inlineBoxSize };
logicalRect.moveBy(LayoutPoint { lineBoxLogicalRect.topLeft() });
if (inlineBoxStartSet.contains(&layoutBox)) {
// This inline box showed up first on this line.
- logicalRect.moveHorizontally(boxGeometry.marginStart());
boxGeometry.setLogicalTopLeft(logicalRect.topLeft());
auto contentBoxHeight = logicalRect.height() - (boxGeometry.verticalBorder() + boxGeometry.verticalPadding().valueOr(0_lu));
boxGeometry.setContentBoxHeight(contentBoxHeight);
auto contentBoxWidth = logicalRect.width() - (boxGeometry.horizontalBorder() + boxGeometry.horizontalPadding().valueOr(0_lu));
- if (inlineBoxEndSet.contains(&layoutBox)) {
- // This is a single line inline box.
- contentBoxWidth -= std::max(0_lu, boxGeometry.marginStart()) + std::max(0_lu, boxGeometry.marginEnd());
- } else
- contentBoxWidth -= std::max(0_lu, boxGeometry.marginStart());
boxGeometry.setContentBoxWidth(contentBoxWidth);
continue;
}
// Middle or end of the inline box. Let's stretch the box as needed.
- if (inlineBoxEndSet.contains(&layoutBox)) {
- // The inline box ends on this line e.g. <span>fist<br>middle<br>last line</span>
- logicalRect.expandHorizontally(-std::max(0_lu, boxGeometry.marginEnd()));
- }
auto enclosingBorderBoxRect = BoxGeometry::borderBoxRect(boxGeometry);
enclosingBorderBoxRect.expandToContain(logicalRect);
boxGeometry.setLogicalLeft(enclosingBorderBoxRect.left());
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp (273222 => 273223)
--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp 2021-02-21 21:08:03 UTC (rev 273222)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp 2021-02-21 21:38:41 UTC (rev 273223)
@@ -301,7 +301,8 @@
ascent = downcast<ReplacedBox>(layoutBox).baseline().valueOr(marginBoxHeight);
else
ascent = marginBoxHeight;
- auto atomicInlineLevelBox = LineBox::InlineLevelBox::createAtomicInlineLevelBox(layoutBox, logicalLeft, { run.logicalWidth(), marginBoxHeight });
+ logicalLeft += std::max(0_lu, inlineLevelBoxGeometry.marginStart());
+ auto atomicInlineLevelBox = LineBox::InlineLevelBox::createAtomicInlineLevelBox(layoutBox, logicalLeft, { inlineLevelBoxGeometry.borderBoxWidth(), marginBoxHeight });
atomicInlineLevelBox->setBaseline(ascent);
atomicInlineLevelBox->setLayoutBounds(LineBox::InlineLevelBox::LayoutBounds { ascent, marginBoxHeight - ascent });
// Let's pre-compute the logical top so that we can avoid running the alignment on simple inline boxes.
@@ -335,8 +336,14 @@
// e.g. <div><span></span><span></span></div> is still okay.
m_inlineLevelBoxesNeedVerticalAlignment = m_inlineLevelBoxesNeedVerticalAlignment || lineHasContent;
if (run.isInlineBoxStart()) {
+ // At this point we don't know yet how wide this inline box is. Let's assume it's as long as the line is
+ // and adjust it later if we come across an inlineBoxEnd run (see below).
auto initialLogicalWidth = lineBox.contentLogicalWidth() - run.logicalLeft();
ASSERT(initialLogicalWidth >= 0);
+ // Inline box run is based on margin box. Let's convert it to border box.
+ auto marginStart = std::max(0_lu, formattingContext().geometryForBox(layoutBox).marginStart());
+ logicalLeft += marginStart;
+ initialLogicalWidth -= marginStart;
auto inlineBox = LineBox::InlineLevelBox::createInlineBox(layoutBox, logicalLeft, initialLogicalWidth);
setVerticalGeometryForInlineBox(*inlineBox);
lineBox.addInlineLevelBox(WTFMove(inlineBox));
@@ -346,7 +353,9 @@
// Adjust the logical width when the inline level container closes on this line.
auto& inlineBox = lineBox.inlineLevelBoxForLayoutBox(layoutBox);
ASSERT(inlineBox.isInlineBox());
- auto inlineBoxLogicalRight = logicalLeft + run.logicalWidth();
+ // Inline box run is based on margin box. Let's convert it to border box.
+ auto marginEnd = std::max(0_lu, formattingContext().geometryForBox(layoutBox).marginEnd());
+ auto inlineBoxLogicalRight = logicalLeft + run.logicalWidth() - marginEnd;
inlineBox.setLogicalWidth(inlineBoxLogicalRight - inlineBox.logicalLeft());
continue;
}
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.cpp (273222 => 273223)
--- trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.cpp 2021-02-21 21:08:03 UTC (rev 273222)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.cpp 2021-02-21 21:38:41 UTC (rev 273223)
@@ -152,20 +152,24 @@
return InlineRect { inlineBoxAbsolutelogicalTop, inlineBoxLogicalRect.left(), inlineBoxLogicalRect.width(), inlineBoxLogicalRect.height() };
}
-InlineRect LineBox::logicalMarginRectForAtomicInlineLevelBox(const Box& layoutBox) const
+InlineRect LineBox::logicalBorderBoxForAtomicInlineLevelBox(const Box& layoutBox, const BoxGeometry& boxGeometry) const
{
ASSERT(layoutBox.isAtomicInlineLevelBox());
- return logicalRectForInlineLevelBox(layoutBox);
+ auto logicalRect = logicalRectForInlineLevelBox(layoutBox);
+ // Inline level boxes use their margin box for vertical alignment. Let's covert them to border boxes.
+ logicalRect.moveVertically(boxGeometry.marginBefore());
+ auto verticalMargin = boxGeometry.marginBefore() + boxGeometry.marginAfter();
+ logicalRect.expandVertically(-verticalMargin);
+ return logicalRect;
}
-InlineRect LineBox::logicalRectForInlineBox(const Box& layoutBox, const BoxGeometry& boxGeometry) const
+InlineRect LineBox::logicalBorderBoxForInlineBox(const Box& layoutBox, const BoxGeometry& boxGeometry) const
{
auto logicalRect = logicalRectForInlineLevelBox(layoutBox);
- // This logical rect is as tall as the "text" content is. Let's adjust with vertical border and padding -vertical margin is ignored.
+ // This logical rect is as tall as the "text" content is. Let's adjust with vertical border and padding.
auto verticalBorderAndPadding = boxGeometry.verticalBorder() + boxGeometry.verticalPadding().valueOr(0_lu);
logicalRect.expandVertically(verticalBorderAndPadding);
logicalRect.moveVertically(-(boxGeometry.borderTop() + boxGeometry.paddingTop().valueOr(0_lu)));
- // This is essentially margin box rect without the vertical margins.
return logicalRect;
}
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.h (273222 => 273223)
--- trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.h 2021-02-21 21:08:03 UTC (rev 273222)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.h 2021-02-21 21:38:41 UTC (rev 273223)
@@ -122,6 +122,7 @@
private:
WeakPtr<const Box> m_layoutBox;
+ // This is the combination of margin and border boxes. Inline level boxes are vertically aligned using their margin boxes.
InlineRect m_logicalRect;
LayoutBounds m_layoutBounds;
InlineLayoutUnit m_baseline { 0 };
@@ -151,9 +152,9 @@
InlineRect logicalRectForTextRun(const Line::Run&) const;
InlineRect logicalRectForLineBreakBox(const Box&) const;
- InlineRect logicalMarginRectForAtomicInlineLevelBox(const Box&) const;
InlineRect logicalRectForRootInlineBox() const { return m_rootInlineBox->logicalRect(); }
- InlineRect logicalRectForInlineBox(const Box&, const BoxGeometry&) const;
+ InlineRect logicalBorderBoxForAtomicInlineLevelBox(const Box&, const BoxGeometry&) const;
+ InlineRect logicalBorderBoxForInlineBox(const Box&, const BoxGeometry&) const;
const InlineLevelBox& rootInlineBox() const { return *m_rootInlineBox; }
using InlineLevelBoxList = Vector<std::unique_ptr<InlineLevelBox>>;
Modified: trunk/Source/WebCore/layout/integration/LayoutIntegrationInlineContentBuilder.cpp (273222 => 273223)
--- trunk/Source/WebCore/layout/integration/LayoutIntegrationInlineContentBuilder.cpp 2021-02-21 21:08:03 UTC (rev 273222)
+++ trunk/Source/WebCore/layout/integration/LayoutIntegrationInlineContentBuilder.cpp 2021-02-21 21:38:41 UTC (rev 273223)
@@ -228,12 +228,11 @@
auto& layoutBox = lineRun.layoutBox();
auto lineIndex = lineRun.lineIndex();
auto& lineBoxLogicalRect = lines[lineIndex].lineBoxLogicalRect();
- // Inline boxes are relative to the line box while final Runs need to be relative to the parent Box
+ // Inline boxes are relative to the line box while final runs need to be relative to the parent box
// FIXME: Shouldn't we just leave them be relative to the line box?
auto runRect = FloatRect { lineRun.logicalRect() };
- // Line runs are margin box based, let's convert them to border box.
auto& geometry = m_layoutState.geometryForBox(layoutBox);
- runRect.moveBy({ lineBoxLogicalRect.left() + std::max(geometry.marginStart(), 0_lu), lineBoxLogicalRect.top() + geometry.marginBefore() });
+ runRect.moveBy({ lineBoxLogicalRect.left(), lineBoxLogicalRect.top() });
runRect.setSize({ geometry.borderBoxWidth(), geometry.borderBoxHeight() });
if (lineLevelVisualAdjustmentsForRuns[lineIndex].needsIntegralPosition)
runRect.setY(roundToInt(runRect.y()));
@@ -248,8 +247,6 @@
auto lineIndex = lineRun.lineIndex();
auto& lineBoxLogicalRect = lines[lineIndex].lineBoxLogicalRect();
auto runRect = FloatRect { lineRun.logicalRect() };
- // Inline boxes are relative to the line box while final Runs need to be relative to the parent Box
- // FIXME: Shouldn't we just leave them be relative to the line box?
runRect.moveBy({ lineBoxLogicalRect.left(), lineBoxLogicalRect.top() });
if (lineLevelVisualAdjustmentsForRuns[lineIndex].needsIntegralPosition)
runRect.setY(roundToInt(runRect.y()));
@@ -378,7 +375,7 @@
continue;
auto& layoutBox = inlineLevelBox->layoutBox();
auto& boxGeometry = m_layoutState.geometryForBox(layoutBox);
- auto inlineBoxRect = lineBox.logicalRectForInlineBox(layoutBox, boxGeometry);
+ auto inlineBoxRect = lineBox.logicalBorderBoxForInlineBox(layoutBox, boxGeometry);
inlineBoxRect.moveBy(lineBoxLogicalRect.topLeft());
inlineContent.nonRootInlineBoxes.append({ lineIndex, layoutBox, inlineBoxRect, inlineQuirks.inlineLevelBoxAffectsLineBox(*inlineLevelBox, lineBox) });
Modified: trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp (273222 => 273223)
--- trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp 2021-02-21 21:08:03 UTC (rev 273222)
+++ trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp 2021-02-21 21:38:41 UTC (rev 273223)
@@ -413,13 +413,13 @@
logicalRect = lineBox.logicalRectForRootInlineBox();
} else if (inlineLevelBox.isAtomicInlineLevelBox()) {
stream << "Atomic inline level box";
- logicalRect = lineBox.logicalMarginRectForAtomicInlineLevelBox(layoutBox);
+ logicalRect = lineBox.logicalBorderBoxForAtomicInlineLevelBox(layoutBox, layoutState.geometryForBox(layoutBox));
} else if (inlineLevelBox.isLineBreakBox()) {
stream << "Line break box";
logicalRect = lineBox.logicalRectForLineBreakBox(layoutBox);
} else if (inlineLevelBox.isInlineBox()) {
stream << "Inline box";
- logicalRect = lineBox.logicalRectForInlineBox(layoutBox, layoutState.geometryForBox(layoutBox));
+ logicalRect = lineBox.logicalBorderBoxForInlineBox(layoutBox, layoutState.geometryForBox(layoutBox));
} else
stream << "Generic inline level box";
stream
