Attachment 29951 Details for Bug 25135 – second try

[patch] second try

place_ellipsis.patch (text/plain), 30.49 KB, created by Jeremy Moskovich on 2009-05-01 15:51:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Moskovich

Created: 2009-05-01 15:51:47 PDT

Size: 30.49 KB

patch

obsolete

>commit 796a472f3159e026ef0091e0594d5d598ea9ef5e
>Author: Jeremy Moskovich <jeremy@chromium.org>
>Date:   Fri May 1 15:21:38 2009 -0700
>
>    - Implement text-overflow:ellipsis for cases where the direction of the last run
>    
>    is different from the overall flow direction - bug 25135.
>    - Make decoration drawing in InlineFlowbox RTL-aware - bug 24206.
>    - Set full trancation on InlineBoxes correctly in the case of RTL flow - bug 24186
>    - Add tests for the above + reorder/cleanup tests for easier interpretation of outcome.
>    - Add some FIXME comments in places where it looks like we might need special handing for RTL.
>    
>    https://bugs.webkit.org/show_bug.cgi?id=24186
>    https://bugs.webkit.org/show_bug.cgi?id=25135
>    https://bugs.webkit.org/show_bug.cgi?id=24206
>
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ba810df..690c899 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,37 @@
>+2009-05-01  Jeremy Moskovich  <jeremy@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fix a number of bugs relating to RTL and text-overflow:ellipsis.
>+        Also refactor the relevant code to make it a little clearer.
>+
>+        This CL fixes 3 fundamental issues:
>+        - Corrects behavior if the truncated InlineTextBox's directionality is
>+          different from the overall flow direction - bug 25135.
>+        - Make decoration drawing in InlineFlowbox RTL-aware - bug 24206.
>+        - Full truncation on InlineBoxes in RTL flow - bug 24186
>+        - Add tests for the above + reorder/cleanup tests for easier
>+          interpretation of outcome.
>+
>+        The code for placing ellipsis has been refactored to use
>+        left/right notation rather than a single variable whose meaning
>+        could differ based on flow directionality.
>+
>+        To support differing inline directionality vs flow directionality,
>+        different variables are now used in InlineTextBox::placeEllipsisBox to
>+        track the two.
>+
>+        The drawing mode for underlines in standards mode now correctly handles
>+        RTL truncation and relevant test where added.
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=24186
>+        https://bugs.webkit.org/show_bug.cgi?id=25135
>+        https://bugs.webkit.org/show_bug.cgi?id=24206
>+
>+        * fast/css/text-overflow-ellipsis-bidi.html: Added.
>+        * fast/css/text-overflow-ellipsis-strict.html: Added.
>+        * fast/css/text-overflow-ellipsis.html:
>+
> 2009-04-30  Kevin McCullough  <kmccullough@apple.com>
> 
>         Reviewed by Adele Peterson and Darin Adler.
>diff --git a/LayoutTests/fast/css/text-overflow-ellipsis-bidi.html b/LayoutTests/fast/css/text-overflow-ellipsis-bidi.html
>new file mode 100644
>index 0000000..f023b13
>--- /dev/null
>+++ b/LayoutTests/fast/css/text-overflow-ellipsis-bidi.html
>@@ -0,0 +1,47 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<style type="text/css">
>+.testBlock {
>+  display:block;
>+  overflow: hidden;
>+  white-space: nowrap;
>+  width: 207px;
>+}
>+
>+.ellipsis {
>+  text-overflow: ellipsis;
>+}
>+</style>
>+</head>
>+<body>
>+Each consecutive pair of lines should look exactly the same apart from the trailing ellipsis.<br>
>+The ellipsis should appear on the left for the RTL case and on the right for LTR case.
>+
>+<br><br>
>+
>+RTL text:
>+<div class="testBlock"dir=rtl>
>+&#x202B;
>+English Text ××§×¡× ×¢××¨× ×××××× ××§×¡× ×¢××¨× ××××××
>+&#x202c;
>+</div>
>+<div class="testBlock ellipsis"dir=rtl>
>+&#x202B;
>+English Text ××§×¡× ×¢××¨× ×××××× ××§×¡× ×¢××¨× ××××××
>+&#x202c;
>+</div>
>+<br>
>+LTR Text
>+<div class="testBlock"dir=ltr>
>+×××× word ×××¨×ª another ××¢×× yet ×××ª
>+</div>
>+
>+<div class="testBlock ellipsis"dir=ltr>
>+×××× word ×××¨×ª another ××¢×× yet ×××ª
>+</div>
>+
>+
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/css/text-overflow-ellipsis-strict.html b/LayoutTests/fast/css/text-overflow-ellipsis-strict.html
>new file mode 100644
>index 0000000..516ad26
>--- /dev/null
>+++ b/LayoutTests/fast/css/text-overflow-ellipsis-strict.html
>@@ -0,0 +1,94 @@
>+<!DOCTYPE html>
>+<head>
>+<meta charset="utf-8">
>+<style>
>+.testDiv {
>+    width: 180px;
>+    height: 20px;
>+    border: 1px solid black;
>+    white-space: nowrap;
>+    overflow: hidden;
>+    margin: 0 0 20px 0;
>+}
>+
>+.forcertl {
>+    direction: rtl;
>+    unicode-bidi: bidi-override;
>+}
>+
>+.ellipsis {
>+    text-overflow:ellipsis;
>+}
>+</style>
>+</head>
>+<body>
>+
>+This tests checks that the text-overflow:ellipsis property is handled correctly
>+for bidi text.<br><br>
>+
>+Pass if the ellipsis do not overlap the text and the underlines for the links are
>+placed under the text and not the ellipsis.
>+<br><br>
>+
>+RTL:
>+<div class="testDiv ellipsis" dir=rtl>
>+××©×××§×¡× ×¢×¨×× ××× × ×¨×¦× ××××¡××£ ××××¤×¡××ª ××¡××£ ×©××¨×
>+</div>
>+
>+RTL Override:
>+<div class="testDiv forcertl ellipsis" dir=rtl>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</div>
>+
>+RTL link:
>+<div class="testDiv forcertl ellipsis" dir=rtl>
>+<a href=#>Lorem ipsum dolor sit amet, consectetur adipiscing</a>
>+</div>
>+
>+RTL with ltr content:
>+<div class="testDiv ellipsis" dir=rtl>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</div>
>+
>+RTL link with ltr content:
>+<div class="testDiv ellipsis" dir=rtl>
>+<a href=#>Lorem ipsum dolor sit amet, consectetur adipiscing</a>
>+</div>
>+
>+LTR link:
>+<div class="testDiv ellipsis" dir=ltr>
>+<a href=#>Lorem ipsum dolor sit amet, consectetur adipiscing</a>
>+</div>
>+
>+LTR:
>+<div class="testDiv ellipsis" dir=ltr>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</div>
>+
>+LTR with rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+××©×××§×¡× ×¢×¨×× ××× × ×¨×¦× ××××¡××£ ××××¤×¡××ª ××¡××£ ×©××¨×
>+</div>
>+
>+LTR with forced rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+<span class=forcertl dir=rtl>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</span>
>+</div>
>+
>+LTR link with rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+<a href=#>
>+××©×××§×¡× ×¢×¨×× ××× × ×¨×¦× ××××¡××£ ××××¤×¡××ª ××¡××£ ×©××¨×
>+</a>
>+</div>
>+
>+LTR link with forced rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+<a href=#>
>+<span class=forcertl dir=rtl>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</span>
>+</a>
>+</div>
>\ No newline at end of file
>diff --git a/LayoutTests/fast/css/text-overflow-ellipsis.html b/LayoutTests/fast/css/text-overflow-ellipsis.html
>index 213cac5..5770ed5 100644
>--- a/LayoutTests/fast/css/text-overflow-ellipsis.html
>+++ b/LayoutTests/fast/css/text-overflow-ellipsis.html
>@@ -10,51 +10,85 @@
>     margin: 0 0 20px 0;
> }
> 
>-.rtl {
>-    direction: rtl;
>-}
>-
> .forcertl {
>     direction: rtl;
>     unicode-bidi: bidi-override;
> }
> 
>-.ellipses {
>+.ellipsis {
>     text-overflow:ellipsis;
> }
> </style>
> </head>
> <body>
> 
>-This tests checks that the text-overflow:ellipses property is handled correctly
>+This tests checks that the text-overflow:ellipsis property is handled correctly
> for bidi text.<br><br>
> 
>-Pass if the ellipses do not overlap the text and the underlines for the links are
>-placed under the text and not the ellipses.
>+Pass if the ellipsis do not overlap the text and the underlines for the links are
>+placed under the text and not the ellipsis.
> <br><br>
> 
>-LTR:
>-<div class="testDiv ellipses">
>-Lorem ipsum dolor sit amet, consectetur adipiscing
>-</div>
>-
> RTL:
>-<div class="testDiv rtl ellipses">
>+<div class="testDiv ellipsis" dir=rtl>
> ××©×××§×¡× ×¢×¨×× ××× × ×¨×¦× ××××¡××£ ××××¤×¡××ª ××¡××£ ×©××¨×
> </div>
> 
> RTL Override:
>-<div class="testDiv forcertl ellipses">
>+<div class="testDiv forcertl ellipsis" dir=rtl>
> Lorem ipsum dolor sit amet, consectetur adipiscing
> </div>
> 
>+RTL link:
>+<div class="testDiv forcertl ellipsis" dir=rtl>
>+<a href=#>Lorem ipsum dolor sit amet, consectetur adipiscing</a>
>+</div>
>+
>+RTL with ltr content:
>+<div class="testDiv ellipsis" dir=rtl>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</div>
> 
>-LTR link:
>-<div class="testDiv ellipses">
>+RTL link with ltr content:
>+<div class="testDiv ellipsis" dir=rtl>
> <a href=#>Lorem ipsum dolor sit amet, consectetur adipiscing</a>
> </div>
> 
>-RTL link:
>-<div class="testDiv forcertl ellipses">
>+LTR link:
>+<div class="testDiv ellipsis" dir=ltr>
> <a href=#>Lorem ipsum dolor sit amet, consectetur adipiscing</a>
> </div>
>+
>+LTR:
>+<div class="testDiv ellipsis" dir=ltr>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</div>
>+
>+LTR with rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+××©×××§×¡× ×¢×¨×× ××× × ×¨×¦× ××××¡××£ ××××¤×¡××ª ××¡××£ ×©××¨×
>+</div>
>+
>+
>+LTR with forced rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+<span class=forcertl dir=rtl>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</span>
>+</div>
>+
>+LTR link with rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+<a href=#>
>+××©×××§×¡× ×¢×¨×× ××× × ×¨×¦× ××××¡××£ ××××¤×¡××ª ××¡××£ ×©××¨×
>+</a>
>+</div>
>+
>+LTR link with forced rtl content:
>+<div class="testDiv ellipsis" dir=ltr>
>+<a href=#>
>+<span class=forcertl dir=rtl>
>+Lorem ipsum dolor sit amet, consectetur adipiscing
>+</span>
>+</a>
>+</div>
>\ No newline at end of file
>diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
>index 502fffe..2e78f17 100644
>--- a/WebCore/ChangeLog
>+++ b/WebCore/ChangeLog
>@@ -1,3 +1,56 @@
>+2009-05-01  Jeremy Moskovich  <jeremy@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fix a number of bugs relating to RTL and text-overflow:ellipsis.
>+        Also refactor the relevant code to make it a little clearer.
>+
>+        This CL fixes 3 fundamental issues:
>+        - Corrects behavior if the truncated InlineTextBox's directionality is
>+          different from the overall flow direction - bug 25135.
>+        - Make decoration drawing in InlineFlowbox RTL-aware - bug 24206.
>+        - Full truncation on InlineBoxes in RTL flow - bug 24186
>+        - Add tests for the above + reorder/cleanup tests for easier
>+          interpretation of outcome.
>+
>+        The code for placing ellipsis has been refactored to use
>+        left/right notation rather than a single variable whose meaning
>+        could differ based on flow directionality.
>+
>+        To support differing inline directionality vs flow directionality,
>+        different variables are now used in InlineTextBox::placeEllipsisBox to
>+        track the two.
>+
>+        The drawing mode for underlines in standards mode now correctly handles
>+        RTL truncation and relevant test where added.
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=24186
>+        https://bugs.webkit.org/show_bug.cgi?id=25135
>+        https://bugs.webkit.org/show_bug.cgi?id=24206
>+
>+        Tests: fast/css/text-overflow-ellipsis-bidi.html
>+               fast/css/text-overflow-ellipsis-strict.html
>+
>+        * rendering/InlineBox.cpp:
>+        (WebCore::InlineBox::placeEllipsisBox):
>+        * rendering/InlineBox.h:
>+        * rendering/InlineFlowBox.cpp:
>+        (WebCore::InlineFlowBox::paintTextDecorations):
>+        (WebCore::InlineFlowBox::placeEllipsisBox):
>+        * rendering/InlineFlowBox.h:
>+        * rendering/InlineTextBox.cpp:
>+        (WebCore::InlineTextBox::placeEllipsisBox):
>+        (WebCore::InlineTextBox::paint):
>+        * rendering/InlineTextBox.h:
>+        * rendering/RenderFlexibleBox.cpp:
>+        (WebCore::RenderFlexibleBox::layoutVerticalBox):
>+        * rendering/RootInlineBox.cpp:
>+        (WebCore::RootInlineBox::placeEllipsis):
>+        (WebCore::RootInlineBox::placeEllipsisBox):
>+        * rendering/RootInlineBox.h:
>+        * rendering/bidi.cpp:
>+        (WebCore::RenderBlock::checkLinesForTextOverflow):
>+
> 2009-05-01  Dimitri Glazkov  <dglazkov@chromium.org>
> 
>         Reviewed by Eric Seidel.
>diff --git a/WebCore/rendering/InlineBox.cpp b/WebCore/rendering/InlineBox.cpp
>index 9bb3692..2d956a8 100644
>--- a/WebCore/rendering/InlineBox.cpp
>+++ b/WebCore/rendering/InlineBox.cpp
>@@ -280,7 +280,7 @@ bool InlineBox::canAccommodateEllipsis(bool ltr, int blockEdge, int ellipsisWidt
>     return !(boxRect.intersects(ellipsisRect));
> }
> 
>-int InlineBox::placeEllipsisBox(bool, int, int, bool&)
>+int InlineBox::placeEllipsisBox(bool, int, int, int, bool&)
> {
>     // Use -1 to mean "we didn't set the position."
>     return -1;
>diff --git a/WebCore/rendering/InlineBox.h b/WebCore/rendering/InlineBox.h
>index 51080cb..ec8c340 100644
>--- a/WebCore/rendering/InlineBox.h
>+++ b/WebCore/rendering/InlineBox.h
>@@ -224,7 +224,8 @@ public:
>     virtual RenderObject::SelectionState selectionState();
> 
>     virtual bool canAccommodateEllipsis(bool ltr, int blockEdge, int ellipsisWidth);
>-    virtual int placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool&);
>+    // visibleLeftEdge, visibleRightEdge are in the parent's coordinate system.
>+    virtual int placeEllipsisBox(bool ltr, int visibleLeftEdge, int visibleRightEdge, int ellipsisWidth, bool&);
> 
>     void setHasBadParent();
> 
>diff --git a/WebCore/rendering/InlineFlowBox.cpp b/WebCore/rendering/InlineFlowBox.cpp
>index 9e18713..0432a4c 100644
>--- a/WebCore/rendering/InlineFlowBox.cpp
>+++ b/WebCore/rendering/InlineFlowBox.cpp
>@@ -885,18 +885,37 @@ void InlineFlowBox::paintTextDecorations(RenderObject::PaintInfo& paintInfo, int
>         int w = m_width - (borderLeft() + paddingLeft() + borderRight() + paddingRight());
>         RootInlineBox* rootLine = root();
>         if (rootLine->ellipsisBox()) {
>-            int ellipsisX = rootLine->ellipsisBox()->x();
>+            int ellipsisX = m_x + rootLine->ellipsisBox()->x();
>             int ellipsisWidth = rootLine->ellipsisBox()->width();
>-            
>-            // FIXME: Will need to work with RTL
>+            bool ltr = renderer()->style()->direction() == LTR;
>             if (rootLine == this) {
>-                if (x + w >= ellipsisX + ellipsisWidth)
>-                    w -= (x + w - ellipsisX - ellipsisWidth);
>+                // Trim w and x so that the underline isn't drawn underneath the ellipsis.
>+                // ltr: is our right edge farther right than the right edge of the ellipsis.
>+                // rtl: is the left edge of our box farther left than the left edge of the ellipsis.
>+                bool ltrTruncation = ltr && (x + w >= ellipsisX + ellipsisWidth);
>+                bool rtlTruncation = !ltr && (x <= ellipsisX + ellipsisWidth);
>+                if (ltrTruncation)
>+                    w -= (x + w) - (ellipsisX + ellipsisWidth);
>+                else if (rtlTruncation) {
>+                    int dx = m_x - ((ellipsisX - m_x) + ellipsisWidth);
>+                    tx -= dx;
>+                    w += dx;
>+                }
>             } else {
>-                if (x >= ellipsisX)
>+                bool ltrPastEllipsis = ltr && x >= ellipsisX;
>+                bool rtlPastEllipsis = !ltr && (x + w) <= (ellipsisX + ellipsisWidth);
>+                if (ltrPastEllipsis || rtlPastEllipsis)
>                     return;
>-                if (x + w >= ellipsisX)
>+
>+                bool ltrTruncation = ltr && x + w >= ellipsisX;
>+                bool rtlTruncation = !ltr && x <= ellipsisX;
>+                if (ltrTruncation)
>                     w -= (x + w - ellipsisX);
>+                else if (rtlTruncation) {
>+                    int dx = m_x - ((ellipsisX - m_x) + ellipsisWidth);
>+                    tx -= dx;
>+                    w += dx;
>+                }
>             }
>         }
> 
>@@ -1022,13 +1041,32 @@ bool InlineFlowBox::canAccommodateEllipsis(bool ltr, int blockEdge, int ellipsis
>     return true;
> }
> 
>-int InlineFlowBox::placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool& foundBox)
>+int InlineFlowBox::placeEllipsisBox(bool ltr, int blockLeftEdge, int blockRightEdge, int ellipsisWidth, bool& foundBox)
> {
>     int result = -1;
>-    for (InlineBox *box = firstChild(); box; box = box->nextOnLine()) {
>-        int currResult = box->placeEllipsisBox(ltr, blockEdge, ellipsisWidth, foundBox);
>+    // We iterate over all children, the foundBox variable tells us when we've found the
>+    // box containing the ellipsis.  All boxes after that one in the flow are hidden.
>+    // If our flow is ltr then iterate over the boxes from left to right, otherwise iterate
>+    // from right to left. Varying the order allows us to correctly hide the boxes following the ellipsis.
>+    InlineBox *box = ltr ? firstChild() : lastChild();
>+
>+    // NOTE: these will cross after foundBox = true.
>+    int visibleLeftEdge = blockLeftEdge;
>+    int visibleRightEdge = blockRightEdge;
>+
>+    while(box) {
>+        int currResult = box->placeEllipsisBox(ltr, visibleLeftEdge, visibleRightEdge, ellipsisWidth, foundBox);
>         if (currResult != -1 && result == -1)
>             result = currResult;
>+
>+        if (ltr) {
>+            visibleLeftEdge += box->width();
>+            box = box->nextOnLine();
>+        }
>+        else {
>+            visibleRightEdge -= box->width();
>+            box = box->prevOnLine();
>+        }
>     }
>     return result;
> }
>diff --git a/WebCore/rendering/InlineFlowBox.h b/WebCore/rendering/InlineFlowBox.h
>index eedc45b..2462eba 100644
>--- a/WebCore/rendering/InlineFlowBox.h
>+++ b/WebCore/rendering/InlineFlowBox.h
>@@ -141,7 +141,7 @@ public:
>     virtual RenderObject::SelectionState selectionState();
> 
>     virtual bool canAccommodateEllipsis(bool ltr, int blockEdge, int ellipsisWidth);
>-    virtual int placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool&);
>+    virtual int placeEllipsisBox(bool ltr, int blockLeftEdge, int blockRightEdge, int ellipsisWidth, bool&);
> 
>     bool hasTextChildren() const { return m_hasTextChildren; }
> 
>diff --git a/WebCore/rendering/InlineTextBox.cpp b/WebCore/rendering/InlineTextBox.cpp
>index 0952d87..cc7c2d6 100644
>--- a/WebCore/rendering/InlineTextBox.cpp
>+++ b/WebCore/rendering/InlineTextBox.cpp
>@@ -128,20 +128,21 @@ void InlineTextBox::attachLine()
>     toRenderText(renderer())->attachTextBox(this);
> }
> 
>-int InlineTextBox::placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool& foundBox)
>+int InlineTextBox::placeEllipsisBox(bool flowIsLTR, int visibleLeftEdge, int visibleRightEdge, int ellipsisWidth, bool& foundBox)
> {
>     if (foundBox) {
>         m_truncation = cFullTruncation;
>         return -1;
>     }
> 
>-    int ellipsisX = ltr ? blockEdge - ellipsisWidth : blockEdge + ellipsisWidth;
>+    // For LTR this is the left edge of the box, for RTL, the right edge in parent coordinates.
>+    int ellipsisX = flowIsLTR ? visibleRightEdge - ellipsisWidth : visibleLeftEdge + ellipsisWidth;
>     
>     // Criteria for full truncation:
>     // LTR: the left edge of the ellipsis is to the left of our text run.
>     // RTL: the right edge of the ellipsis is to the right of our text run.
>-    bool ltrFullTruncation = ltr && ellipsisX <= m_x;
>-    bool rtlFullTruncation = !ltr && ellipsisX >= (m_x + m_width);
>+    bool ltrFullTruncation = flowIsLTR && ellipsisX <= m_x;
>+    bool rtlFullTruncation = !flowIsLTR && ellipsisX >= (m_x + m_width);
>     if (ltrFullTruncation || rtlFullTruncation) {
>         // Too far.  Just set full truncation, but return -1 and let the ellipsis just be placed at the edge of the box.
>         m_truncation = cFullTruncation;
>@@ -149,14 +150,21 @@ int InlineTextBox::placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth,
>         return -1;
>     }
> 
>-    bool ltrEllipsisWithinBox = ltr && (ellipsisX < m_x + m_width);
>-    bool rtlEllipsisWithinBox = !ltr && (ellipsisX > m_x);
>+    bool ltrEllipsisWithinBox = flowIsLTR && (ellipsisX < m_x + m_width);
>+    bool rtlEllipsisWithinBox = !flowIsLTR && (ellipsisX > m_x);
>     if (ltrEllipsisWithinBox || rtlEllipsisWithinBox) {
>-        if ((ltr && direction() == RTL) || (!ltr && direction() == LTR))
>-            return -1; // FIXME: Support cases in which the last run's directionality differs from the context.
>-
>         foundBox = true;
> 
>+        // The inline box may have different directionality than it's parent.  Since truncation
>+        // behavior depends both on both the parent and the inline block's directionality, we
>+        // must keep track of these separately.
>+        bool ltr = direction() == LTR;
>+        if (ltr != flowIsLTR) {
>+          // Width in pixels of the visible portion of the box, excluding the ellipsis.
>+          int visibleBoxWidth = visibleRightEdge - visibleLeftEdge  - ellipsisWidth;
>+          ellipsisX = ltr ? m_x + visibleBoxWidth : m_x + m_width - visibleBoxWidth;
>+        }
>+
>         int offset = offsetForPosition(ellipsisX, false);
>         if (offset == 0) {
>             // No characters should be rendered.  Set ourselves to full truncation and place the ellipsis at the min of our start
>@@ -165,12 +173,22 @@ int InlineTextBox::placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth,
>             return min(ellipsisX, m_x);
>         }
> 
>-        // Set the truncation index on the text run.  The ellipsis needs to be placed just after the last visible character.
>+        // Set the truncation index on the text run.
>         m_truncation = offset;
>-        if (ltr)
>-            return m_x + toRenderText(renderer())->width(m_start, offset, textPos(), m_firstLine);
>+
>+        // If we got here that means that we were only partially truncated and we need to return the pixel offset at which
>+        // to place the ellipsis.
>+        int widthOfVisibleText = toRenderText(renderer())->width(m_start, offset, textPos(), m_firstLine);
>+
>+        // The ellipsis needs to be placed just after the last visible character.
>+        // Where "after" is defined by the flow directionality, not the inline
>+        // box directionality.
>+        // e.g. In the case of an LTR inline box truncated in an RTL flow then we can
>+        // have a situation such as |Hello| -> |...He|
>+        if (flowIsLTR)
>+            return m_x + widthOfVisibleText;
>         else
>-            return m_x + (m_width - toRenderText(renderer())->width(m_start, offset, textPos(), m_firstLine)) - ellipsisWidth;
>+            return (m_x + m_width) - widthOfVisibleText - ellipsisWidth;
>     }
>     return -1;
> }
>@@ -285,7 +303,7 @@ void InlineTextBox::paint(RenderObject::PaintInfo& paintInfo, int tx, int ty)
>     if (isLineBreak() || !renderer()->shouldPaintWithinRoot(paintInfo) || renderer()->style()->visibility() != VISIBLE ||
>         m_truncation == cFullTruncation || paintInfo.phase == PaintPhaseOutline)
>         return;
>-    
>+
>     ASSERT(paintInfo.phase != PaintPhaseSelfOutline && paintInfo.phase != PaintPhaseChildOutlines);
> 
>     int xPos = tx + m_x - parent()->maxHorizontalVisualOverflow();
>@@ -301,6 +319,24 @@ void InlineTextBox::paint(RenderObject::PaintInfo& paintInfo, int tx, int ty)
>         // When only painting the selection, don't bother to paint if there is none.
>         return;
> 
>+    if (m_truncation != cNoTruncation) {
>+        TextDirection flowDirection = renderer()->containingBlock()->style()->direction();
>+        if (flowDirection != 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|
>+            int widthOfVisibleText = toRenderText(renderer())->width(m_start, m_truncation, textPos(), m_firstLine);
>+            int widthOfHiddenText = m_width - widthOfVisibleText;
>+            // FIXME: The hit testing logic also needs to take this translation int account.
>+            tx += direction() == LTR ? widthOfHiddenText : -widthOfHiddenText;
>+        }
>+    }
>+
>     GraphicsContext* context = paintInfo.context;
> 
>     // Determine whether or not we have composition underlines to draw.
>diff --git a/WebCore/rendering/InlineTextBox.h b/WebCore/rendering/InlineTextBox.h
>index 2fe232e..55d75ab 100644
>--- a/WebCore/rendering/InlineTextBox.h
>+++ b/WebCore/rendering/InlineTextBox.h
>@@ -88,7 +88,7 @@ public:
> 
> private:
>     virtual void clearTruncation() { m_truncation = cNoTruncation; }
>-    virtual int placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool& foundBox);
>+    virtual int placeEllipsisBox(bool flowIsLTR, int visibleLeftEdge, int visibleRightEdge, int ellipsisWidth, bool& foundBox);
> 
> public:
>     virtual bool isLineBreak() const;
>diff --git a/WebCore/rendering/RenderFlexibleBox.cpp b/WebCore/rendering/RenderFlexibleBox.cpp
>index f0d5606..1dcef36 100644
>--- a/WebCore/rendering/RenderFlexibleBox.cpp
>+++ b/WebCore/rendering/RenderFlexibleBox.cpp
>@@ -771,15 +771,19 @@ void RenderFlexibleBox::layoutVerticalBox(bool relayoutChildren)
>                     continue;
>                 if (destBlock->style()->direction() != LTR)
>                     continue;
>+                int ltr = true;
> 
>-                int blockEdge = destBlock->rightOffset(lastVisibleLine->y(), false);
>-                if (!lastVisibleLine->canAccommodateEllipsis(true, blockEdge, 
>+                int blockRightEdge = destBlock->rightOffset(lastVisibleLine->y(), false);
>+                int blockLeftEdge = destBlock->leftOffset(lastVisibleLine->y(), false);
>+
>+                int blockEdge = ltr ? blockRightEdge : blockLeftEdge;
>+                if (!lastVisibleLine->canAccommodateEllipsis(ltr, blockEdge,
>                                                              lastVisibleLine->x() + lastVisibleLine->width(),
>                                                              totalWidth))
>                     continue;
> 
>                 // Let the truncation code kick in.
>-                lastVisibleLine->placeEllipsis(ellipsisAndSpaceStr, true, blockEdge, totalWidth, anchorBox);
>+                lastVisibleLine->placeEllipsis(ellipsisAndSpaceStr, ltr, blockLeftEdge, blockRightEdge, totalWidth, anchorBox);
>                 destBlock->setHasMarkupTruncation(true);
>             }
>         }
>diff --git a/WebCore/rendering/RootInlineBox.cpp b/WebCore/rendering/RootInlineBox.cpp
>index 1fb5174..ed125ee 100644
>--- a/WebCore/rendering/RootInlineBox.cpp
>+++ b/WebCore/rendering/RootInlineBox.cpp
>@@ -99,7 +99,7 @@ bool RootInlineBox::canAccommodateEllipsis(bool ltr, int blockEdge, int lineBoxE
>     return InlineFlowBox::canAccommodateEllipsis(ltr, blockEdge, ellipsisWidth);
> }
> 
>-void RootInlineBox::placeEllipsis(const AtomicString& ellipsisStr,  bool ltr, int blockEdge, int ellipsisWidth,
>+void RootInlineBox::placeEllipsis(const AtomicString& ellipsisStr,  bool ltr, int blockLeftEdge, int blockRightEdge, int ellipsisWidth,
>                                   InlineBox* markupBox)
> {
>     // Create an ellipsis box.
>@@ -113,7 +113,8 @@ void RootInlineBox::placeEllipsis(const AtomicString& ellipsisStr,  bool ltr, in
>     gEllipsisBoxMap->add(this, ellipsisBox);
>     m_hasEllipsisBox = true;
> 
>-    if (ltr && (x() + width() + ellipsisWidth) <= blockEdge) {
>+    // FIXME: Do we need an RTL version of this?
>+    if (ltr && (x() + width() + ellipsisWidth) <= blockRightEdge) {
>         ellipsisBox->m_x = x() + width();
>         return;
>     }
>@@ -122,14 +123,14 @@ void RootInlineBox::placeEllipsis(const AtomicString& ellipsisStr,  bool ltr, in
>     // of that glyph.  Mark all of the objects that intersect the ellipsis box as not painting (as being
>     // truncated).
>     bool foundBox = false;
>-    ellipsisBox->m_x = placeEllipsisBox(ltr, blockEdge, ellipsisWidth, foundBox);
>+    ellipsisBox->m_x = placeEllipsisBox(ltr, blockLeftEdge, blockRightEdge, ellipsisWidth, foundBox);
> }
> 
>-int RootInlineBox::placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool& foundBox)
>+int RootInlineBox::placeEllipsisBox(bool ltr, int blockLeftEdge, int blockRightEdge, int ellipsisWidth, bool& foundBox)
> {
>-    int result = InlineFlowBox::placeEllipsisBox(ltr, blockEdge, ellipsisWidth, foundBox);
>+    int result = InlineFlowBox::placeEllipsisBox(ltr, blockLeftEdge, blockRightEdge, ellipsisWidth, foundBox);
>     if (result == -1)
>-        result = ltr ? blockEdge - ellipsisWidth : blockEdge;
>+        result = ltr ? blockRightEdge - ellipsisWidth : blockLeftEdge;
>     return result;
> }
> 
>diff --git a/WebCore/rendering/RootInlineBox.h b/WebCore/rendering/RootInlineBox.h
>index 1f2a88c..5aebc5c 100644
>--- a/WebCore/rendering/RootInlineBox.h
>+++ b/WebCore/rendering/RootInlineBox.h
>@@ -85,8 +85,8 @@ public:
>     void childRemoved(InlineBox* box);
> 
>     bool canAccommodateEllipsis(bool ltr, int blockEdge, int lineBoxEdge, int ellipsisWidth);
>-    void placeEllipsis(const AtomicString& ellipsisStr, bool ltr, int blockEdge, int ellipsisWidth, InlineBox* markupBox = 0);
>-    virtual int placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool& foundBox);
>+    void placeEllipsis(const AtomicString& ellipsisStr, bool ltr, int blockLeftEdge, int blockRightEdge, int ellipsisWidth, InlineBox* markupBox = 0);
>+    virtual int placeEllipsisBox(bool ltr, int blockLeftEdge, int blockRightEdge, int ellipsisWidth, bool& foundBox);
> 
>     EllipsisBox* ellipsisBox() const;
> 
>diff --git a/WebCore/rendering/bidi.cpp b/WebCore/rendering/bidi.cpp
>index c4d9246..3659912 100644
>--- a/WebCore/rendering/bidi.cpp
>+++ b/WebCore/rendering/bidi.cpp
>@@ -2221,16 +2221,18 @@ void RenderBlock::checkLinesForTextOverflow()
>     // Include the scrollbar for overflow blocks, which means we want to use "contentWidth()"
>     bool ltr = style()->direction() == LTR;
>     for (RootInlineBox* curr = firstRootBox(); curr; curr = curr->nextRootBox()) {
>-        int blockEdge = ltr ? rightOffset(curr->y(), curr == firstRootBox()) : leftOffset(curr->y(), curr == firstRootBox());
>+        int blockRightEdge = rightOffset(curr->y(), curr == firstRootBox());
>+        int blockLeftEdge = leftOffset(curr->y(), curr == firstRootBox());
>         int lineBoxEdge = ltr ? curr->x() + curr->width() : curr->x();
>-        if ((ltr && lineBoxEdge > blockEdge) || (!ltr && lineBoxEdge < blockEdge)) {
>+        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
>             // can be truncated.  In order for truncation to be possible, the line must have sufficient space to
>             // accommodate our truncation string, and no replaced elements (images, tables) can overlap the ellipsis
>             // space.
>             int width = curr == firstRootBox() ? firstLineEllipsisWidth : ellipsisWidth;
>+            int blockEdge = ltr ? blockRightEdge : blockLeftEdge;
>             if (curr->canAccommodateEllipsis(ltr, blockEdge, lineBoxEdge, width))
>-                curr->placeEllipsis(ellipsisStr, ltr, blockEdge, width);
>+                curr->placeEllipsis(ellipsisStr, ltr, blockLeftEdge, blockRightEdge, width);
>         }
>     }
> }

Flags:

hyatt: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 25135: 29513 | 29951