Ellipsis renders multiple times when position: relative and top i…

…s used https://bugs.webkit.org/show_bug.cgi?id=274087 <rdar://problem/128394449> Reviewed by Antti Koivisto. Let's not paint line ending ellipsis when painting is meant to include inline box (with layer) content only. * LayoutTests/fast/inline/multiple-ellispsis-rendering-when-inline-box-has-layer-expected.html: Added. * LayoutTests/fast/inline/multiple-ellispsis-rendering-when-inline-box-has-layer.html: Added. * Source/WebCore/layout/integration/inline/LayoutIntegrationInlineContentPainter.cpp: (WebCore::LayoutIntegration::InlineContentPainter::InlineContentPainter): (WebCore::LayoutIntegration::InlineContentPainter::paint): (WebCore::LayoutIntegration::LayerPaintScope::LayerPaintScope): (WebCore::LayoutIntegration::LayerPaintScope::includes): * Source/WebCore/layout/integration/inline/LayoutIntegrationInlineContentPainter.h: Canonical link: https://commits.webkit.org/279645@main
WebKit · Jun 3, 2024 · 15bbd64 · 15bbd64
1 parent c55e92e
commit 15bbd64
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 19 deletions.
diff --git a/LayoutTests/fast/inline/multiple-ellispsis-rendering-when-inline-box-has-layer-expected.html b/LayoutTests/fast/inline/multiple-ellispsis-rendering-when-inline-box-has-layer-expected.html
@@ -0,0 +1,18 @@
+<style>
+div {
+  padding-top: 20px;
+  font-family: Monospace;
+  font-size: 20px;
+
+  white-space: nowrap;
+  width: 420px;
+  height: 100px;
+  overflow: hidden;
+  text-overflow: ellipsis;
+}
+
+span {
+  color: transparent;
+}
+</style>
+<div class="ellipsis"><span>PASS if there's only one ellipsis on this line</span></div>
diff --git a/LayoutTests/fast/inline/multiple-ellispsis-rendering-when-inline-box-has-layer.html b/LayoutTests/fast/inline/multiple-ellispsis-rendering-when-inline-box-has-layer.html
@@ -0,0 +1,20 @@
+<style>
+.ellipsis {
+  padding-top: 20px;
+  font-family: Monospace;
+  font-size: 20px;
+
+  white-space: nowrap;
+  width: 420px;
+  height: 100px;
+  overflow: hidden;
+  text-overflow: ellipsis;
+}
+
+span {
+  top: -5px;
+  color: transparent;
+  position: relative;
+}
+</style>
+<div class="ellipsis"><span>PASS if there's only one ellipsis on this line</span></div>
diff --git a/Source/WebCore/layout/integration/inline/LayoutIntegrationInlineContentPainter.cpp b/Source/WebCore/layout/integration/inline/LayoutIntegrationInlineContentPainter.cpp
@@ -37,10 +37,10 @@
 namespace WebCore {
 namespace LayoutIntegration {
 
-InlineContentPainter::InlineContentPainter(PaintInfo& paintInfo, const LayoutPoint& paintOffset, const RenderInline* layerRenderer, const InlineContent& inlineContent, const BoxTree& boxTree)
+InlineContentPainter::InlineContentPainter(PaintInfo& paintInfo, const LayoutPoint& paintOffset, const RenderInline* inlineBoxWithLayer, const InlineContent& inlineContent, const BoxTree& boxTree)
     : m_paintInfo(paintInfo)
     , m_paintOffset(paintOffset)
-    , m_layerRenderer(layerRenderer)
+    , m_inlineBoxWithLayer(inlineBoxWithLayer)
     , m_inlineContent(inlineContent)
     , m_boxTree(boxTree)
 {
@@ -107,9 +107,21 @@ void InlineContentPainter::paintDisplayBox(const InlineDisplay::Box& box)
 
 void InlineContentPainter::paint()
 {
-    auto layerPaintScope = LayerPaintScope { m_boxTree, m_layerRenderer };
+    auto layerPaintScope = LayerPaintScope { m_boxTree, m_inlineBoxWithLayer };
     auto lastBoxLineIndex = std::optional<size_t> { };
 
+    auto paintLineEndingEllipsisIfApplicable = [&](std::optional<size_t> currentLineIndex) {
+        // Since line ending ellipsis belongs to the line structure but we don't have the concept of painting the line itself
+        // let's paint it when we are either at the end of the content or finished painting a line with ellipsis.
+        // While normally ellipsis is on the last line, -webkit-line-clamp can make us put ellipsis on any line.
+        if (m_inlineBoxWithLayer) {
+            // Line ending ellipsis is never on the inline box (with layer).
+            return;
+        }
+        if (lastBoxLineIndex && (!currentLineIndex || *lastBoxLineIndex != currentLineIndex))
+            paintEllipsis(*lastBoxLineIndex);
+    };
+
     for (auto& box : m_inlineContent.boxesForRect(m_damageRect)) {
         auto shouldPaintBoxForPhase = [&] {
             switch (m_paintInfo.phase) {
@@ -127,18 +139,12 @@ void InlineContentPainter::paint()
         };
 
         if (shouldPaintBoxForPhase() && layerPaintScope.includes(box)) {
-            auto finishedPaintingLine = lastBoxLineIndex && *lastBoxLineIndex != box.lineIndex();
-            if (finishedPaintingLine) {
-                // Paint the ellipsis as the line item on the previous line.
-                paintEllipsis(*lastBoxLineIndex);
-            }
+            paintLineEndingEllipsisIfApplicable(box.lineIndex());
             paintDisplayBox(box);
         }
         lastBoxLineIndex = box.lineIndex();
     }
-
-    if (lastBoxLineIndex)
-        paintEllipsis(*lastBoxLineIndex);
+    paintLineEndingEllipsisIfApplicable({ });
 
     for (auto& renderInline : m_outlineObjects)
         renderInline.paintOutline(m_paintInfo, m_paintOffset);
@@ -151,9 +157,9 @@ LayoutPoint InlineContentPainter::flippedContentOffsetIfNeeded(const RenderBox&
     return m_paintOffset;
 }
 
-LayerPaintScope::LayerPaintScope(const BoxTree& boxTree, const RenderInline* layerRenderer)
+LayerPaintScope::LayerPaintScope(const BoxTree& boxTree, const RenderInline* inlineBoxWithLayer)
     : m_boxTree(boxTree)
-    , m_layerInlineBox(layerRenderer ? layerRenderer->layoutBox() : nullptr)
+    , m_inlineBoxWithLayer(inlineBoxWithLayer ? inlineBoxWithLayer->layoutBox() : nullptr)
 {
 }
 
@@ -173,9 +179,9 @@ bool LayerPaintScope::includes(const InlineDisplay::Box& box)
         return false;
     };
 
-    if (m_layerInlineBox == &box.layoutBox())
+    if (m_inlineBoxWithLayer == &box.layoutBox())
         return true;
-    if (m_layerInlineBox && !isInside(box, *m_layerInlineBox))
+    if (m_inlineBoxWithLayer && !isInside(box, *m_inlineBoxWithLayer))
         return false;
     if (m_currentExcludedInlineBox && isInside(box, *m_currentExcludedInlineBox))
         return false;

diff --git a/Source/WebCore/layout/integration/inline/LayoutIntegrationInlineContentPainter.h b/Source/WebCore/layout/integration/inline/LayoutIntegrationInlineContentPainter.h
@@ -52,7 +52,7 @@ struct InlineContent;
 
 class InlineContentPainter {
 public:
-    InlineContentPainter(PaintInfo&, const LayoutPoint& paintOffset, const RenderInline* layerRenderer, const InlineContent&, const BoxTree&);
+    InlineContentPainter(PaintInfo&, const LayoutPoint& paintOffset, const RenderInline* inlineBoxWithLayer, const InlineContent&, const BoxTree&);
 
     void paint();
 
@@ -65,20 +65,20 @@ class InlineContentPainter {
     PaintInfo& m_paintInfo;
     const LayoutPoint m_paintOffset;
     LayoutRect m_damageRect;
-    const RenderInline* m_layerRenderer { nullptr };
+    const RenderInline* m_inlineBoxWithLayer { nullptr };
     const InlineContent& m_inlineContent;
     const BoxTree& m_boxTree;
     SingleThreadWeakListHashSet<RenderInline> m_outlineObjects;
 };
 
 class LayerPaintScope {
 public:
-    LayerPaintScope(const BoxTree&, const RenderInline* layerRenderer);
+    LayerPaintScope(const BoxTree&, const RenderInline* inlineBoxWithLayer);
     bool includes(const InlineDisplay::Box&);
 
 private:
     const BoxTree& m_boxTree;
-    const Layout::ElementBox* const m_layerInlineBox;
+    const Layout::ElementBox* const m_inlineBoxWithLayer;
     const Layout::ElementBox* m_currentExcludedInlineBox { nullptr };
 };