From 3001e5dd0235ad99a1d9caa08ab7b5ce37008754 Mon Sep 17 00:00:00 2001 From: Alan Baradlay Date: Wed, 26 Apr 2023 10:03:56 -0700 Subject: [PATCH] text-overflow: ellipsis truncates the text incorrectly in RTL https://bugs.webkit.org/show_bug.cgi?id=164999 Reviewed by Antti Koivisto. This patch addresses https://github.com/w3c/csswg-drafts/issues/2125 which clarifies truncation behavior when content direction != inline direction. truncateOverflowingDisplayBoxes: mismatching direction requires truncation starting from the beginning of the content. paintForegroundAndDecorations: since TextBoxSelectableRange only has the truncated length, we need to adjust the position for mismatching direction. * LayoutTests/TestExpectations: 27 and 28 have also progressed but we seem to have some rounding bugs getting an extra character truncated (Chrome too). * Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp: (WebCore::Layout::truncateOverflowingDisplayBoxes): * Source/WebCore/rendering/TextBoxPainter.cpp: (WebCore::TextBoxPainter::paintForegroundAndDecorations): (WebCore::TextBoxPainter::computePaintRect): Canonical link: https://commits.webkit.org/263418@main --- LayoutTests/TestExpectations | 1 - ...ext_overflow_ellipsis_simple-expected.html | 8 +- .../display/InlineDisplayLineBuilder.cpp | 110 ++++++++++++------ Source/WebCore/rendering/TextBoxPainter.cpp | 30 ++--- .../rendering/TextBoxSelectableRange.h | 1 + 5 files changed, 90 insertions(+), 60 deletions(-) diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations index 4b93952a4b48..f3306c875c29 100644 --- a/LayoutTests/TestExpectations +++ b/LayoutTests/TestExpectations @@ -3305,7 +3305,6 @@ imported/w3c/web-platform-tests/css/css-display/run-in/run-in-text-between-004.x imported/w3c/web-platform-tests/css/css-display/run-in/run-in-text-between-005.xht [ ImageOnlyFailure ] imported/w3c/web-platform-tests/css/css-ui/text-overflow-027.html [ ImageOnlyFailure ] imported/w3c/web-platform-tests/css/css-ui/text-overflow-028.html [ ImageOnlyFailure ] -imported/w3c/web-platform-tests/css/css-ui/text-overflow-029.html [ ImageOnlyFailure ] imported/w3c/web-platform-tests/css/mediaqueries/viewport-script-dynamic.html [ ImageOnlyFailure ] diff --git a/LayoutTests/fast/inline/text_overflow_ellipsis_simple-expected.html b/LayoutTests/fast/inline/text_overflow_ellipsis_simple-expected.html index 7349f41f40eb..68382a144c0a 100644 --- a/LayoutTests/fast/inline/text_overflow_ellipsis_simple-expected.html +++ b/LayoutTests/fast/inline/text_overflow_ellipsis_simple-expected.html @@ -10,7 +10,7 @@
AB…
AB…
AB…
-
EF…
-
EF…
-
EF…
-
EF…
+
GH…
+
GH…
+
GH…
+
GH…
diff --git a/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp index 2f7f73ed0e82..2f6616dea64d 100644 --- a/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp +++ b/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp @@ -153,11 +153,80 @@ InlineDisplay::Line InlineDisplayLineBuilder::build(const LineBuilder::LineConte }; } +static float truncateTextContentWithMismatchingDirection(InlineDisplay::Box& displayBox, float contentWidth, float availableWidthForContent, bool canFullyTruncate) +{ + // While truncation normally starts at the end of the content and progress backwards, with mismatching direction + // we take a different approach and truncate content the other way around (i.e. ellipsis follows inline direction truncating the beginning of the content). + //
some long content
+ // [...ng content] + auto& inlineTextBox = downcast(displayBox.layoutBox()); + auto& textContent = displayBox.text(); + + auto availableWidthForTruncatedContent = contentWidth - availableWidthForContent; + auto truncatedSide = TextUtil::breakWord(inlineTextBox, textContent.start(), textContent.length(), contentWidth, availableWidthForTruncatedContent, { }, displayBox.style().fontCascade()); + + ASSERT(truncatedSide.length < textContent.length()); + auto visibleLength = textContent.length() - truncatedSide.length; + auto visibleWidth = contentWidth - truncatedSide.logicalWidth; + // TextUtil::breakWord never returns overflowing content (left side) which means the right side is normally wider (by one character) than what we need. + auto visibleContentOverflows = truncatedSide.logicalWidth < availableWidthForTruncatedContent; + if (!visibleContentOverflows) { + textContent.setPartiallyVisibleContentLength(visibleLength); + return visibleWidth; + } + + auto wouldFullyTruncate = visibleLength <= 1; + if (wouldFullyTruncate && canFullyTruncate) { + displayBox.setIsFullyTruncated(); + return { }; + } + + visibleLength = !wouldFullyTruncate ? visibleLength - 1 : 1; + textContent.setPartiallyVisibleContentLength(visibleLength); + auto visibleStartPosition = textContent.end() - visibleLength; + return TextUtil::width(inlineTextBox, displayBox.style().fontCascade(), visibleStartPosition, textContent.end(), { }, TextUtil::UseTrailingWhitespaceMeasuringOptimization::No); +} + +static float truncate(InlineDisplay::Box& displayBox, float contentWidth, float availableWidthForContent, bool canFullyTruncate) +{ + if (displayBox.isText()) { + if (!availableWidthForContent && canFullyTruncate) { + displayBox.setIsFullyTruncated(); + return { }; + } + auto contentDirection = displayBox.bidiLevel() % 2 ? TextDirection::RTL : TextDirection::LTR; + if (displayBox.layoutBox().parent().style().direction() != contentDirection) + return truncateTextContentWithMismatchingDirection(displayBox, contentWidth, availableWidthForContent, canFullyTruncate); + + auto& inlineTextBox = downcast(displayBox.layoutBox()); + auto& textContent = displayBox.text(); + auto visibleSide = TextUtil::breakWord(inlineTextBox, textContent.start(), textContent.length(), contentWidth, availableWidthForContent, { }, displayBox.style().fontCascade()); + if (visibleSide.length) { + textContent.setPartiallyVisibleContentLength(visibleSide.length); + return visibleSide.logicalWidth; + } + if (canFullyTruncate) { + displayBox.setIsFullyTruncated(); + return { }; + } + auto firstCharacterLength = TextUtil::firstUserPerceivedCharacterLength(inlineTextBox, textContent.start(), textContent.length()); + auto firstCharacterWidth = TextUtil::width(inlineTextBox, displayBox.style().fontCascade(), textContent.start(), textContent.start() + firstCharacterLength, { }, TextUtil::UseTrailingWhitespaceMeasuringOptimization::No); + textContent.setPartiallyVisibleContentLength(firstCharacterLength); + return firstCharacterWidth; + } + + if (canFullyTruncate) { + // Atomic inline level boxes are never partially truncated. + displayBox.setIsFullyTruncated(); + return { }; + } + return contentWidth; +} + static float truncateOverflowingDisplayBoxes(const InlineDisplay::Line& displayLine, InlineDisplay::Boxes& boxes, float ellipsisWidth, const RenderStyle& rootStyle) { // We gotta truncate some runs. ASSERT(displayLine.lineBoxLogicalRect().x() + displayLine.contentLogicalLeft() + displayLine.contentLogicalWidth() + ellipsisWidth > displayLine.lineBoxLogicalRect().maxX()); - auto isLeftToRightDirection = rootStyle.isLeftToRightDirection(); auto isHorizontal = rootStyle.isHorizontalWritingMode(); auto left = [&] (auto& displayBox) { return isHorizontal ? displayBox.left() : displayBox.top(); @@ -168,47 +237,17 @@ static float truncateOverflowingDisplayBoxes(const InlineDisplay::Line& displayL auto width = [&] (auto& displayBox) { return isHorizontal ? displayBox.width() : displayBox.height(); }; - auto truncate = [&] (auto& displayBox, auto visibleWidth, auto canFullyTruncate) { - if (displayBox.isText()) { - if (!visibleWidth && canFullyTruncate) { - displayBox.setIsFullyTruncated(); - return isLeftToRightDirection ? left(displayBox) : right(displayBox); - } - - auto& inlineTextBox = downcast(displayBox.layoutBox()); - auto& textContent = displayBox.text(); - auto leftSide = TextUtil::breakWord(inlineTextBox, textContent.start(), textContent.length(), width(displayBox), visibleWidth, { }, displayBox.style().fontCascade()); - if (leftSide.length) { - textContent.setPartiallyVisibleContentLength(leftSide.length); - return isLeftToRightDirection ? left(displayBox) + leftSide.logicalWidth : right(displayBox) - leftSide.logicalWidth; - } - if (canFullyTruncate) { - displayBox.setIsFullyTruncated(); - return isLeftToRightDirection ? left(displayBox) : right(displayBox); - } - auto firstCharacterLength = TextUtil::firstUserPerceivedCharacterLength(inlineTextBox, textContent.start(), textContent.length()); - auto firstCharacterWidth = TextUtil::width(inlineTextBox, displayBox.style().fontCascade(), textContent.start(), textContent.start() + firstCharacterLength, { }, TextUtil::UseTrailingWhitespaceMeasuringOptimization::No); - textContent.setPartiallyVisibleContentLength(firstCharacterLength); - return isLeftToRightDirection ? left(displayBox) + firstCharacterWidth : right(displayBox) - firstCharacterWidth; - } - if (canFullyTruncate) { - // Atomic inline level boxes are never partially truncated. - displayBox.setIsFullyTruncated(); - return isLeftToRightDirection ? left(displayBox) : right(displayBox); - } - return isLeftToRightDirection ? right(displayBox) : left(displayBox); - }; - // The logically first character or atomic inline-level element on a line must be clipped rather than ellipsed. auto isFirstContentRun = true; - if (isLeftToRightDirection) { + if (rootStyle.isLeftToRightDirection()) { auto visualRightForContentEnd = std::max(0.f, right(displayLine) - ellipsisWidth); auto truncateRight = std::optional { }; for (auto& displayBox : boxes) { if (displayBox.isInlineBox()) continue; if (right(displayBox) > visualRightForContentEnd) { - auto truncatePosition = truncate(displayBox, std::max(0.f, visualRightForContentEnd - left(displayBox)), !isFirstContentRun); + auto visibleWidth = truncate(displayBox, width(displayBox), std::max(0.f, visualRightForContentEnd - left(displayBox)), !isFirstContentRun); + auto truncatePosition = left(displayBox) + visibleWidth; truncateRight = truncateRight.value_or(truncatePosition); } isFirstContentRun = false; @@ -223,7 +262,8 @@ static float truncateOverflowingDisplayBoxes(const InlineDisplay::Line& displayL if (displayBox.isInlineBox()) continue; if (left(displayBox) < visualLeftForContentEnd) { - auto truncatePosition = truncate(displayBox, std::max(0.f, right(displayBox) - visualLeftForContentEnd), !isFirstContentRun); + auto visibleWidth = truncate(displayBox, width(displayBox), std::max(0.f, right(displayBox) - visualLeftForContentEnd), !isFirstContentRun); + auto truncatePosition = right(displayBox) - visibleWidth; truncateLeft = truncateLeft.value_or(truncatePosition); } isFirstContentRun = false; diff --git a/Source/WebCore/rendering/TextBoxPainter.cpp b/Source/WebCore/rendering/TextBoxPainter.cpp index 089b8e5dad2a..d02d35280307 100644 --- a/Source/WebCore/rendering/TextBoxPainter.cpp +++ b/Source/WebCore/rendering/TextBoxPainter.cpp @@ -245,6 +245,8 @@ void TextBoxPainter::paintForegroundAndDecorations() auto hasTextDecoration = !m_style.textDecorationsInEffect().isEmpty(); auto hasHighlightDecoration = m_document.hasHighlight() && !MarkedText::collectForHighlights(m_renderer, m_selectableRange, MarkedText::PaintPhase::Decoration).isEmpty(); auto hasDecoration = hasTextDecoration || hasHighlightDecoration; + auto hasMismatchingContentDirection = m_renderer.containingBlock()->style().direction() != textBox().direction(); + auto hasBackwardTrunctation = m_selectableRange.truncation && hasMismatchingContentDirection; auto contentMayNeedStyledMarkedText = [&] { if (hasDecoration) @@ -257,9 +259,15 @@ void TextBoxPainter::paintForegroundAndDecorations() return true; return false; }; + auto startPosition = [&] { + return !hasBackwardTrunctation ? m_selectableRange.clamp(textBox().start()) : textBox().length() - *m_selectableRange.truncation; + }; + auto endPosition = [&] { + return !hasBackwardTrunctation ? m_selectableRange.clamp(textBox().end()) : textBox().length(); + }; if (!contentMayNeedStyledMarkedText()) { auto& lineStyle = m_isFirstLine ? m_renderer.firstLineStyle() : m_renderer.style(); - auto markedText = MarkedText { m_selectableRange.clamp(textBox().start()), m_selectableRange.clamp(textBox().end()), MarkedText::Unmarked }; + auto markedText = MarkedText { startPosition(), endPosition(), MarkedText::Unmarked }; auto styledMarkedText = StyledMarkedText { markedText, StyledMarkedText::computeStyleForUnmarkedMarkedText(m_renderer, lineStyle, m_isFirstLine, m_paintInfo) }; paintCompositionForeground(styledMarkedText); return; @@ -268,7 +276,7 @@ void TextBoxPainter::paintForegroundAndDecorations() Vector markedTexts; if (m_paintInfo.phase != PaintPhase::Selection) { // The marked texts for the gaps between document markers and selection are implicitly created by subdividing the entire line. - markedTexts.append({ m_selectableRange.clamp(textBox().start()), m_selectableRange.clamp(textBox().end()), MarkedText::Unmarked }); + markedTexts.append({ startPosition(), endPosition(), MarkedText::Unmarked }); if (!m_isPrinting) { markedTexts.appendVector(MarkedText::collectForDocumentMarkers(m_renderer, m_selectableRange, MarkedText::PaintPhase::Foreground)); @@ -843,25 +851,7 @@ FloatRect TextBoxPainter::computePaintRect(const LayoutPoint& paint { FloatPoint localPaintOffset(paintOffset); - if (m_selectableRange.truncation) { - if (m_renderer.containingBlock()->style().direction() != textBox().direction()) { - // Make the visible fragment of text hug the edge closest to the rest of the run by moving the origin - // at which we start drawing text. - // e.g. In the case of LTR text truncated in an RTL Context, the correct behavior is: - // |Hello|CBA| -> |...He|CBA| - // In order to draw the fragment "He" aligned to the right edge of it's box, we need to start drawing - // farther to the right. - // NOTE: WebKit's behavior differs from that of IE which appears to just overlay the ellipsis on top of the - // truncated string i.e. |Hello|CBA| -> |...lo|CBA| - LayoutUnit widthOfVisibleText { m_renderer.width(textBox().start(), *m_selectableRange.truncation, textPosition(), m_isFirstLine) }; - LayoutUnit widthOfHiddenText { m_logicalRect.width() - widthOfVisibleText }; - LayoutSize truncationOffset(textBox().direction() == TextDirection::LTR ? widthOfHiddenText : -widthOfHiddenText, 0_lu); - localPaintOffset.move(textBox().isHorizontal() ? truncationOffset : truncationOffset.transposedSize()); - } - } - localPaintOffset.move(0, m_style.isHorizontalWritingMode() ? 0 : -m_logicalRect.height()); - auto visualRect = textBox().visualRectIgnoringBlockDirection(); textBox().formattingContextRoot().flipForWritingMode(visualRect); diff --git a/Source/WebCore/rendering/TextBoxSelectableRange.h b/Source/WebCore/rendering/TextBoxSelectableRange.h index 7a1c1034b8b3..65c0102ce468 100644 --- a/Source/WebCore/rendering/TextBoxSelectableRange.h +++ b/Source/WebCore/rendering/TextBoxSelectableRange.h @@ -33,6 +33,7 @@ struct TextBoxSelectableRange { const unsigned length; const unsigned additionalLengthAtEnd { 0 }; const bool isLineBreak { false }; + // FIXME: Consider holding onto the truncation position instead. See webkit.org/b/164999 const std::optional truncation { }; unsigned clamp(unsigned offset) const