From fe66ba3f36feab600b09aabea7bd95f9dce3f2cf Mon Sep 17 00:00:00 2001 From: Ojan Vafai Date: Fri, 21 Nov 2014 11:26:05 -0800 Subject: [PATCH] Fix text-overflow:ellipsis. This regressed in 3db9471ae80bd492f2a346113d2323ba8eee0c09. Accidentally sent some code down the override by sending the top LayoutUnit instead of the left/right LayoutUnit. The root problem was bad overloading, which is also fixed in this patch. Inlined the overloaded method since one of the calls was only called from one place. The new test demonstrates the ellipsizing, but that doesn't show through in the render tree dump. We don't get real test coverage here until we either start doing pixel tests or start exposing ellipsis in the render tree dumps. R=esprehn@chromium.org Review URL: https://codereview.chromium.org/751483002 --- engine/core/rendering/RenderBlockFlow.h | 25 ++++++------------- .../core/rendering/RenderBlockLineLayout.cpp | 15 ++++++----- tests/layout/ellipsis-expected.txt | 22 ++++++++++++++++ tests/layout/ellipsis.sky | 25 +++++++++++++++++++ 4 files changed, 62 insertions(+), 25 deletions(-) create mode 100644 tests/layout/ellipsis-expected.txt create mode 100644 tests/layout/ellipsis.sky diff --git a/engine/core/rendering/RenderBlockFlow.h b/engine/core/rendering/RenderBlockFlow.h index 5edac473c89..34806d1d657 100644 --- a/engine/core/rendering/RenderBlockFlow.h +++ b/engine/core/rendering/RenderBlockFlow.h @@ -65,11 +65,17 @@ public: } LayoutUnit logicalRightOffsetForLine(bool shouldIndentText) const { - return logicalRightOffsetForLine(logicalRightOffsetForContent(), shouldIndentText); + LayoutUnit right = logicalRightOffsetForContent(); + if (shouldIndentText && !style()->isLeftToRightDirection()) + right -= textIndentOffset(); + return right; } LayoutUnit logicalLeftOffsetForLine(bool shouldIndentText) const { - return logicalLeftOffsetForLine(logicalLeftOffsetForContent(), shouldIndentText); + LayoutUnit left = logicalLeftOffsetForContent(); + if (shouldIndentText && style()->isLeftToRightDirection()) + left += textIndentOffset(); + return left; } LayoutUnit startOffsetForLine(bool shouldIndentText) const { @@ -125,21 +131,6 @@ protected: void determineLogicalLeftPositionForChild(RenderBox* child); private: - LayoutUnit logicalRightOffsetForLine(LayoutUnit fixedOffset, bool applyTextIndent) const - { - LayoutUnit right = fixedOffset; - if (applyTextIndent && !style()->isLeftToRightDirection()) - right -= textIndentOffset(); - return right; - } - LayoutUnit logicalLeftOffsetForLine(LayoutUnit fixedOffset, bool applyTextIndent) const - { - LayoutUnit left = fixedOffset; - if (applyTextIndent && style()->isLeftToRightDirection()) - left += textIndentOffset(); - return left; - } - void layoutBlockFlow(bool relayoutChildren, SubtreeLayoutScope&); void layoutBlockChildren(bool relayoutChildren, SubtreeLayoutScope&, LayoutUnit beforeEdge, LayoutUnit afterEdge); diff --git a/engine/core/rendering/RenderBlockLineLayout.cpp b/engine/core/rendering/RenderBlockLineLayout.cpp index 50847e08c81..3a19e7bedea 100644 --- a/engine/core/rendering/RenderBlockLineLayout.cpp +++ b/engine/core/rendering/RenderBlockLineLayout.cpp @@ -1208,8 +1208,7 @@ void RenderBlockFlow::layoutInlineChildren(bool relayoutChildren, LayoutUnit& pa // Text truncation kicks in in two cases: // 1) If your overflow isn't visible and your text-overflow-mode isn't clip. - // 2) If you're an anonymous block with a block parent that satisfies #1 that was created - // to accomodate a block that has inline and block children. + // 2) If you're an anonymous paragraph with a parent that satisfies #1. // FIXME: CSS3 says that descendants that are clipped must also know how to truncate. This is insanely // difficult to figure out in general (especially in the middle of doing layout), so we only handle the // simple case of an anonymous block truncating when it's parent is clipped. @@ -1512,8 +1511,8 @@ void RenderBlockFlow::deleteEllipsisLineBoxes() curr->clearTruncation(); // Shift the line back where it belongs if we cannot accomodate an ellipsis. - float logicalLeft = logicalLeftOffsetForLine(curr->lineTop(), firstLine).toFloat(); - float availableLogicalWidth = logicalRightOffsetForLine(curr->lineTop(), false) - logicalLeft; + float logicalLeft = logicalLeftOffsetForLine(firstLine).toFloat(); + float availableLogicalWidth = logicalRightOffsetForLine(false) - logicalLeft; float totalLogicalWidth = curr->logicalWidth(); updateLogicalWidthForAlignment(textAlign, curr, 0, logicalLeft, totalLogicalWidth, availableLogicalWidth, 0); @@ -1547,8 +1546,8 @@ void RenderBlockFlow::checkLinesForTextOverflow() bool firstLine = true; for (RootInlineBox* curr = firstRootBox(); curr; curr = curr->nextRootBox()) { float currLogicalLeft = curr->logicalLeft(); - LayoutUnit blockRightEdge = logicalRightOffsetForLine(curr->lineTop(), firstLine); - LayoutUnit blockLeftEdge = logicalLeftOffsetForLine(curr->lineTop(), firstLine); + LayoutUnit blockRightEdge = logicalRightOffsetForLine(firstLine); + LayoutUnit blockLeftEdge = logicalLeftOffsetForLine(firstLine); LayoutUnit lineBoxEdge = ltr ? currLogicalLeft + curr->logicalWidth() : currLogicalLeft; if ((ltr && lineBoxEdge > blockRightEdge) || (!ltr && lineBoxEdge < blockLeftEdge)) { // This line spills out of our box in the appropriate direction. Now we need to see if the line @@ -1583,8 +1582,8 @@ LayoutUnit RenderBlockFlow::startAlignedOffsetForLine(bool firstLine) // updateLogicalWidthForAlignment() handles the direction of the block so no need to consider it here float totalLogicalWidth = 0; - float logicalLeft = logicalLeftOffsetForLine(logicalHeight(), false).toFloat(); - float availableLogicalWidth = logicalRightOffsetForLine(logicalHeight(), false) - logicalLeft; + float logicalLeft = logicalLeftOffsetForLine(false).toFloat(); + float availableLogicalWidth = logicalRightOffsetForLine(false) - logicalLeft; updateLogicalWidthForAlignment(textAlign, 0, 0, logicalLeft, totalLogicalWidth, availableLogicalWidth, 0); if (!style()->isLeftToRightDirection()) diff --git a/tests/layout/ellipsis-expected.txt b/tests/layout/ellipsis-expected.txt new file mode 100644 index 00000000000..5d3ada6bfa5 --- /dev/null +++ b/tests/layout/ellipsis-expected.txt @@ -0,0 +1,22 @@ +layer at (0,0) size 800x600 + RenderView {#document} at (0,0) size 800x600 +layer at (0,0) size 800x94 + RenderBlock {sky} at (0,0) size 800x94 +layer at (372,0) size 56x69 clip at (375,3) size 50x63 scrollWidth 93 + RenderBlock {div} at (372,0) size 56x69 [border: (3px solid #0000FF)] + RenderBlock (anonymous) at (3,3) size 50x19 + RenderText {#text} at (0,0) size 91x19 + text run at (0,0) width 91: "These lines" + RenderBlock (anonymous) at (3,47) size 50x19 + RenderText {#text} at (0,0) size 94x19 + text run at (0,0) width 94: "all ellipsize." +layer at (375,22) size 56x25 backgroundClip at (375,22) size 50x25 clip at (378,25) size 47x19 outlineClip at (375,3) size 50x63 scrollWidth 53 + RenderBlock {p} at (3,22) size 56x25 [border: (3px solid #FF0000)] + RenderText {#text} at (3,3) size 54x19 + text run at (3,3) width 54: "should" +layer at (372,69) size 56x25 clip at (375,72) size 50x19 scrollWidth 117 + RenderBlock {p} at (372,69) size 56x25 [border: (3px solid #FF0000)] + RenderInline {span} at (0,0) size 117x19 + RenderText {#text} at (3,3) size 117x19 + text run at (3,3) width 117: "As should this." + diff --git a/tests/layout/ellipsis.sky b/tests/layout/ellipsis.sky new file mode 100644 index 00000000000..08288a42a0e --- /dev/null +++ b/tests/layout/ellipsis.sky @@ -0,0 +1,25 @@ + + + +
These lines

should

all ellipsize.
+

As should this.

+