Changeset 282202 in webkit

Timestamp:

Sep 9, 2021, 1:17:06 AM (5 years ago)

Author:

Antti Koivisto

Message:

RenderInline offsetTop/Left should not switch to legacy layout
​https://bugs.webkit.org/show_bug.cgi?id=230040

Reviewed by Alan Bujtas.

LayoutTests/imported/w3c:

• web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt:
• web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt:

Source/WebCore:

Get position of the LFC inline box.

• layout/integration/LayoutIntegrationLineLayout.cpp:

(WebCore::LayoutIntegration::LineLayout::firstInlineBoxRect const):

• layout/integration/LayoutIntegrationLineLayout.h:
• rendering/RenderInline.cpp:

(WebCore::RenderInline::offsetLeft const):
(WebCore::RenderInline::offsetTop const):
(WebCore::RenderInline::firstInlineBoxTopLeft const):
(WebCore::RenderInline::positionForPoint const):

Also fix positionForPoint as a failing test revealed a missing LFC path here.

• rendering/RenderInline.h:

LayoutTests:

• editing/selection/doubleclick-beside-cr-span.html:

Update the test for more standards compliant first inline box position.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/selection/doubleclick-beside-cr-span.html (modified) (2 diffs)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/layout/integration/LayoutIntegrationLineLayout.cpp (modified) (1 diff)
• Source/WebCore/layout/integration/LayoutIntegrationLineLayout.h (modified) (1 diff)
• Source/WebCore/rendering/RenderInline.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderInline.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-09-09  Antti Koivisto  <antti@apple.com>
+
+        RenderInline offsetTop/Left should not switch to legacy layout
+        https://bugs.webkit.org/show_bug.cgi?id=230040
+
+        Reviewed by Alan Bujtas.
+
+        * editing/selection/doubleclick-beside-cr-span.html:
+
+        Update the test for more standards compliant first inline box position.
+
 2021-09-09  Youenn Fablet  <youenn@apple.com>
 

trunk/LayoutTests/editing/selection/doubleclick-beside-cr-span.html

@@ -150 +150 @@
         doTest("totest_tab_out", "select4\t");
         doTest("totest_multiple_whitespaces_out", "select5 ");
-        doTest("totest_linebreak_in", "select6\n");
-        doTest("totest_tab_in", "select7\t");
-        doTest("totest_multiple_whitespaces_in", "select8 ");
+        doTest("totest_linebreak_in", "select6\n", getPositionOfNextLineHeadOf("totest_linebreak_in"));
+        doTest("totest_tab_in", "select7\t", getPositionOfNextLineHeadOf("totest_tab_in"));
+        doTest("totest_multiple_whitespaces_in", "select8 ", getPositionOfNextLineHeadOf("totest_multiple_whitespaces_in"));
 
         doTest("totest_multiple_word_in_span", "select9 ");
@@ -177 +177 @@
         doTest("totest_multiple_whitespaces_out_preline", "select3 ");
         doTest("totest_linebreak_in_preline", "select4\n", getPositionOfNextLineHeadOf("totest_linebreak_in_preline"));
-        doTest("totest_tab_in_preline", "select5\t");
-        doTest("totest_multiple_whitespaces_in_preline", "select6 ");
+        doTest("totest_tab_in_preline", "select5\t", getPositionOfNextLineHeadOf("totest_tab_in_preline"));
+        doTest("totest_multiple_whitespaces_in_preline", "select6 ", getPositionOfNextLineHeadOf("totest_multiple_whitespaces_in_preline"));
 
         doTest("totest_linebreak_out_nowrap", "select1\n");

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2021-09-09  Antti Koivisto  <antti@apple.com>
+
+        RenderInline offsetTop/Left should not switch to legacy layout
+        https://bugs.webkit.org/show_bug.cgi?id=230040
+
+        Reviewed by Alan Bujtas.
+
+        * web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt:
+        * web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt:
+
 2021-09-09  Martin Robinson  <mrobinson@webkit.org>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt

@@ -6 +6 @@
 ref
 
-FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 0 assert_equals: offsetLeft expected 16 but got 0
+PASS offsetTop/Left of empty inline elements should work as if they were not empty: 0
 FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 1 assert_equals: offsetLeft expected 34 but got 0
 FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 2 assert_equals: offsetLeft expected 34 but got 0

trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt

@@ -6 +6 @@
 ref
 
-FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 0 assert_equals: offsetLeft expected 16 but got 0
+PASS offsetTop/Left of empty inline elements should work as if they were not empty: 0
 FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 1 assert_equals: offsetLeft expected 34 but got 0
 FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 2 assert_equals: offsetLeft expected 34 but got 0

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-09-09  Antti Koivisto  <antti@apple.com>
+
+        RenderInline offsetTop/Left should not switch to legacy layout
+        https://bugs.webkit.org/show_bug.cgi?id=230040
+
+        Reviewed by Alan Bujtas.
+
+        Get position of the LFC inline box.
+
+        * layout/integration/LayoutIntegrationLineLayout.cpp:
+        (WebCore::LayoutIntegration::LineLayout::firstInlineBoxRect const):
+        * layout/integration/LayoutIntegrationLineLayout.h:
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::offsetLeft const):
+        (WebCore::RenderInline::offsetTop const):
+        (WebCore::RenderInline::firstInlineBoxTopLeft const):
+        (WebCore::RenderInline::positionForPoint const):
+
+        Also fix positionForPoint as a failing test revealed a missing LFC path here.
+
+        * rendering/RenderInline.h:
+
 2021-09-09  Martin Robinson  <mrobinson@webkit.org>
 

trunk/Source/WebCore/layout/integration/LayoutIntegrationLineLayout.cpp

@@ -412 +412 @@
 }
 
+LayoutRect LineLayout::firstInlineBoxRect(const RenderInline& renderInline) const
+{
+    auto& layoutBox = m_boxTree.layoutBoxForRenderer(renderInline);
+    for (auto& run : m_inlineContent->runs) {
+        if (&run.layoutBox() == &layoutBox)
+            return Layout::toLayoutRect(run.logicalRect());
+    }
+    return { };
+}
+
 LayoutRect LineLayout::enclosingBorderBoxRectFor(const RenderInline& renderInline) const
 {

trunk/Source/WebCore/layout/integration/LayoutIntegrationLineLayout.h

@@ -97 +97 @@
     LineIterator lastLine() const;
 
+    LayoutRect firstInlineBoxRect(const RenderInline&) const;
     LayoutRect enclosingBorderBoxRectFor(const RenderInline&) const;
     LayoutRect visualOverflowBoundingBoxRectFor(const RenderInline&) const;

trunk/Source/WebCore/rendering/RenderInline.cpp

@@ -434 +434 @@
 LayoutUnit RenderInline::offsetLeft() const
 {
+    return adjustedPositionRelativeToOffsetParent(firstInlineBoxTopLeft()).x();
+}
+
+LayoutUnit RenderInline::offsetTop() const
+{
+    return adjustedPositionRelativeToOffsetParent(firstInlineBoxTopLeft()).y();
+}
+
+LayoutPoint RenderInline::firstInlineBoxTopLeft() const
+{
 #if ENABLE(LAYOUT_FORMATTING_CONTEXT)
-    if (auto* lineLayout = LayoutIntegration::LineLayout::containing(const_cast<RenderInline&>(*this)))
-        lineLayout->flow().ensureLineBoxes();
+    if (auto* lineLayout = LayoutIntegration::LineLayout::containing(*this))
+        return lineLayout->firstInlineBoxRect(*this).location();
 #endif
-    LayoutPoint topLeft;
     if (LegacyInlineBox* firstBox = firstLineBoxIncludingCulling())
-        topLeft = flooredLayoutPoint(firstBox->topLeft());
-    return adjustedPositionRelativeToOffsetParent(topLeft).x();
-}
-
-LayoutUnit RenderInline::offsetTop() const
-{
-#if ENABLE(LAYOUT_FORMATTING_CONTEXT)
-    if (auto* lineLayout = LayoutIntegration::LineLayout::containing(const_cast<RenderInline&>(*this)))
-        lineLayout->flow().ensureLineBoxes();
-#endif
-    LayoutPoint topLeft;
-    if (LegacyInlineBox* firstBox = firstLineBoxIncludingCulling())
-        topLeft = flooredLayoutPoint(firstBox->topLeft());
-    return adjustedPositionRelativeToOffsetParent(topLeft).y();
+        return flooredLayoutPoint(firstBox->topLeft());
+    return { };
 }
 
@@ -583 +580 @@
     // FIXME: Does not deal with relative or sticky positioned inlines (should it?)
     RenderBlock& containingBlock = *this->containingBlock();
-    if (firstLineBox()) {
-        // This inline actually has a line box.  We must have clicked in the border/padding of one of these boxes.  We
+
+    auto hasInlineBox = [&] {
+#if ENABLE(LAYOUT_FORMATTING_CONTEXT)
+        if (LayoutIntegration::LineLayout::containing(*this))
+            return true;
+#endif
+        return !!firstLineBox();
+    };
+
+    if (hasInlineBox()) {
+        // This inline actually has an inline box. We must have clicked in the border/padding of one of these boxes. We
         // should try to find a result by asking our containing block.
         return containingBlock.positionForPoint(point, fragment);

trunk/Source/WebCore/rendering/RenderInline.h

@@ -126 +126 @@
     LayoutUnit offsetWidth() const final { return linesBoundingBox().width(); }
     LayoutUnit offsetHeight() const final { return linesBoundingBox().height(); }
+    LayoutPoint firstInlineBoxTopLeft() const;
 
     LayoutRect clippedOverflowRect(const RenderLayerModelObject* repaintContainer, VisibleRectContext) const override;