Bug 25135 - text-overflow:ellipsis doesn't correctly handle cases where a text run and inline box have different directionality
Summary: text-overflow:ellipsis doesn't correctly handle cases where a text run and in...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Jeremy Moskovich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-10 15:20 PDT by Jeremy Moskovich
Modified: 2018-02-02 23:07 PST (History)
4 users (show)

See Also:


Attachments
Proposed fix (20.27 KB, patch)
2009-04-15 12:47 PDT, Jeremy Moskovich
no flags Details | Formatted Diff | Diff
second try (30.49 KB, patch)
2009-05-01 15:51 PDT, Jeremy Moskovich
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Moskovich 2009-04-10 15:20:52 PDT
.e.g.The case of an RTL div where the ellipsis is placed on to of an LTR text run,.
Comment 1 Jeremy Moskovich 2009-04-15 12:47:17 PDT
Created attachment 29513 [details]
Proposed fix

Added code to handle this case and ammended Layout Test appropriately.
Comment 2 Jeremy Moskovich 2009-04-15 12:52:36 PDT
Here's a slightly clearer description of the problem:

.test {
    width: 180px;
    height: 20px;
    border: 1px solid black;
    white-space: nowrap;
    overflow: hidden;
    margin: 0 0 20px 0;
}

.ellipsis {
    text-overflow:ellipsis;
}

<div class="test ellipsis" dir=RTL>
Hello
</div>

The directionality of the div dictates the side that the ellipsis will be drawn on, in this case the left side of the text.

This case differs from what's currently handled in the code since the difference in directionality means we need to truncate from the START of the text run.

Therefore the correct truncation should look something like:
...lo

This is also true for the opposite case (ltr aligned div containg an RTL text run).
Comment 3 mitz 2009-04-15 13:18:45 PDT
> Therefore the correct truncation should look something like:
> ...lo

It does not seem right for truncation to remove text from the middle and leave out the end.
Comment 4 Jeremy Moskovich 2009-04-15 13:29:32 PDT
This is how IE behaves. If you look at the Layout test after this patch is applied you'll see WebKit behaves identically to IE.

Another reference (although, I'm not sure how authoratative), is this post:
http://markmail.org/message/r45kbqerj3sgisc2#query:text-overflow%3Aellipsis%20rtl+page:1+mid:oxndx7i3fnmjumrl+state:results

"Not sure that is what you are saying, but I think there should never be 
anything between the ellipsis and the boundary of the box: You lay out 
the lines using the bidi-algorithm, and then remove characters from the 
end of the *visual* lines:"
Comment 5 Jeremy Moskovich 2009-04-15 14:24:03 PDT
Perhaps this makes more sense to me in the context of multiple directional runs, so for example, let's look at two cases:

The simple case - the directionality of the div and the inline element are identical:
<div class="test ellipsis" dir=LTR>
Hello World
</div>

This truncates to |Hello Wo...|

Now, let's add some RTL text on the end:
<div class="test ellipsis" dir=LTR>
Hello World OLLEH
</div>

There are basically 2 options for truncating this:

1. |Hello World OL...|
This is what the patch I've uploaded and IE do and seems like the right behavior to me.

2. |Hello World ...EH|
Placing the ellipsis in the middle of the run rather than close to the box edge seems wrong to me.

What do you think?
Comment 6 Jeremy Moskovich 2009-04-16 09:58:59 PDT
mitz: Could you confirm my interpretation of your comment so I can be sure we're on the same page before uploading a new patch?

Do you mean that instead of truncating:
Hello World OLLEH -> |Hello World OL...|

We should truncate this as:
Hello World OLLEH -> |Hello World EH...|

?
Comment 7 mitz 2009-04-17 14:47:44 PDT
(In reply to comment #6)
> mitz: Could you confirm my interpretation of your comment so I can be sure
> we're on the same page before uploading a new patch?
> 
> Do you mean that instead of truncating:
> Hello World OLLEH -> |Hello World OL...|
> 
> We should truncate this as:
> Hello World OLLEH -> |Hello World EH...|

Yes, that preserves the beginning of the text rather than removing parts from the middle. 
Comment 8 Jeremy Moskovich 2009-04-22 15:10:22 PDT
Comment on attachment 29513 [details]
Proposed fix

clear review bit, pending update patch.
Comment 9 Jeremy Moskovich 2009-05-01 15:51:47 PDT
Created attachment 29951 [details]
second try

Fix for the issues Mitz raised (thanks!).

I've now bundled this together with the patches for another couple of bugs, since it doesn't really make sense to land them separately.
Comment 10 Dave Hyatt 2009-05-13 13:59:18 PDT
Comment on attachment 29951 [details]
second try

r=me
Comment 11 Dimitri Glazkov (Google) 2009-05-13 15:33:20 PDT
Landed as http://trac.webkit.org/changeset/43664.