Skip to content

Commit

Permalink
text-overflow: ellipsis truncates the text incorrectly in RTL
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=164999
<rdar://problem/29464657>

Reviewed by Antti Koivisto.

This patch addresses w3c/csswg-drafts#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<TextBoxPath>::paintForegroundAndDecorations):
(WebCore::TextBoxPainter<TextBoxPath>::computePaintRect):

Canonical link: https://commits.webkit.org/263418@main
  • Loading branch information
@alanbaradlay
alanbaradlay committed Apr 26, 2023
1 parent bf05a6e commit 3001e5d
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 60 deletions.
1 change: 0 additions & 1 deletion LayoutTests/TestExpectations
View file
Expand Up @@ -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 ]

Expand Down
8 changes: 4 additions & 4 deletions LayoutTests/fast/inline/text_overflow_ellipsis_simple-expected.html
View file
Expand Up @@ -10,7 +10,7 @@
<div style="border-left: 10px solid">AB&#8230;</div>
<div style="border-left: 10px solid; padding-left: 20px;">AB&#8230;</div>
<div style="border: 10px solid; padding: 20px;">AB&#8230;</div>
<div style="direction: rtl;">EF&#8230;</div>
<div style="direction: rtl; border-left: 10px solid">EF&#8230;</div>
<div style="direction: rtl; border-left: 10px solid; padding-left: 20px;">EF&#8230;</div>
<div style="direction: rtl; border: 10px solid; padding: 20px;">EF&#8230;</div>
<div style="direction: rtl;">GH&#8230;</div>
<div style="direction: rtl; border-left: 10px solid">GH&#8230;</div>
<div style="direction: rtl; border-left: 10px solid; padding-left: 20px;">GH&#8230;</div>
<div style="direction: rtl; border: 10px solid; padding: 20px;">GH&#8230;</div>
110 changes: 75 additions & 35 deletions Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp
View file
Expand Up @@ -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).
// <div dir=rtl>some long content</div>
// [...ng content]
auto& inlineTextBox = downcast<InlineTextBox>(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<InlineTextBox>(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();
Expand All @@ -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<InlineTextBox>(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<float> { };
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;
Expand All @@ -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;
Expand Down
30 changes: 10 additions & 20 deletions Source/WebCore/rendering/TextBoxPainter.cpp
View file
Expand Up @@ -245,6 +245,8 @@ void TextBoxPainter<TextBoxPath>::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)
Expand All @@ -257,9 +259,15 @@ void TextBoxPainter<TextBoxPath>::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;
Expand All @@ -268,7 +276,7 @@ void TextBoxPainter<TextBoxPath>::paintForegroundAndDecorations()
Vector<MarkedText> 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));
Expand Down Expand Up @@ -843,25 +851,7 @@ FloatRect TextBoxPainter<TextBoxPath>::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);

Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/rendering/TextBoxSelectableRange.h
View file
Expand Up @@ -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<unsigned> truncation { };

unsigned clamp(unsigned offset) const
Expand Down

0 comments on commit 3001e5d

Please sign in to comment.