Bug 23471

Summary: RTL: text-overflow: ellipses draws ellipses on top of text
Product: WebKit Reporter: Jeremy Moskovich <playmobil>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz, xji
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
an example
none
First cut at layout test
none
Patch version 1
none
Screenshot of problems with patch 1
none
HTML used to generate screenshot
none
Patch version 2 hyatt: review+

Description Jeremy Moskovich 2009-01-21 23:41:49 PST
Text is not properly clipped and ellipses are drawn on top of text when using text-overflow: ellipses in conjunction with dir: rtl.

You can see this in the last example in the test case for the inline-spelling-markers layout test (attached).
Comment 1 Jeremy Moskovich 2009-01-21 23:42:31 PST
Created attachment 26923 [details]
an example
Comment 2 Jeremy Moskovich 2009-01-31 15:45:40 PST
Created attachment 27224 [details]
First cut at layout test

Any suggestions for extra test cases?
Comment 3 Jeremy Moskovich 2009-01-31 21:38:04 PST
Created attachment 27227 [details]
Patch version 1

This is a first cut at a patch, it's not complete but I have some questions before I continue:
* In the case of RTL text, the first ellipse isn't spaced properly from the last letter of the text, what's the correct way to add the space between the text and ellipse?
* What would be the best place to put the accompanying layout test?
* With this patch, RTL links draw an underline only under the ellipses and not the actual text (see attached testcase & screenshot), any ideas where this might stem from?
* Is the check for m_dirOverride ok?  It's not clear to me why the placement of the EllipsesBox is different using a forced bidi override?
Comment 4 Jeremy Moskovich 2009-01-31 21:38:39 PST
Created attachment 27228 [details]
Screenshot of problems with patch 1
Comment 5 Jeremy Moskovich 2009-01-31 21:39:47 PST
Created attachment 27229 [details]
HTML used to generate screenshot
Comment 6 Jeremy Moskovich 2009-02-01 17:09:08 PST
Created attachment 27237 [details]
Patch version 2

This fixes the issues with the previous patch.
Comment 7 Dave Hyatt 2009-02-04 10:12:31 PST
Comment on attachment 27237 [details]
Patch version 2

Looks fine to me.  r=me
Comment 8 Dimitri Glazkov (Google) 2009-02-04 12:34:36 PST
Landed as http://trac.webkit.org/changeset/40620