Bug 17921 - Extra white space at the end of right-aligned or justified text with -webkit-line-break: after-white-space
Summary: Extra white space at the end of right-aligned or justified text with -webkit-...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction
Depends on:
Blocks:
 
Reported: 2008-03-18 13:52 PDT by mitz
Modified: 2008-04-18 14:21 PDT (History)
0 users

See Also:


Attachments
Test case (1.73 KB, text/html)
2008-03-18 13:52 PDT, mitz
no flags Details
Correct trailing whitespace behavior (276.26 KB, patch)
2008-04-18 13:26 PDT, mitz
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2008-03-18 13:52:30 PDT
Changing the white-space and -webkit-line-break properties in the attached test case should not affect the text at all, regardless of the text-align property value. Currently you get extra white space at the end of the lines (or no justification at all).
Comment 1 mitz 2008-03-18 13:52:46 PDT
Created attachment 19875 [details]
Test case
Comment 2 mitz 2008-03-18 13:56:28 PDT
Note that since -webkit-line-break: after-white-space is the default for editable HTML, this affects editing.
Comment 3 mitz 2008-03-18 14:07:00 PDT
Firefox sort of agrees with WebKit's current behavior for pre-wrap, so maybe only -webkit-line-break needs fixing.
Comment 4 mitz 2008-04-18 13:26:49 PDT
Created attachment 20675 [details]
Correct trailing whitespace behavior
Comment 5 Darin Adler 2008-04-18 13:38:38 PDT
Comment on attachment 20675 [details]
Correct trailing whitespace behavior

This code is pretty hard for me to follow, but probably necessarily so.

I'd assert that run->m_previous is 0 in prependRun.

 668                 trailingSpaceWidth =  (min(trailingSpaceRun->m_box->width(), (availableWidth - totWidth + 1) / 2));

This has an extra space and unneeded parentheses.

 949                             if (current != ' ' && current != '\t' && current != softHyphen && (current != '\n' || lastText->style()->preserveNewline()) && (current != noBreakSpace || lastText->style()->nbspMode() == NBNORMAL))

It seems like this line would read better if there was a helper function for it.

 957                             bool shouldSeparateSpaces = style()->direction() == RTL || trailingSpaceRun != start.lastRun() || trailingSpaceRun->m_level % 2 || textAlign != LEFT && textAlign != WEBKIT_LEFT && textAlign != TAAUTO;

This is such a long condition that it might need some comments or be moved into an inline function or something. It also might be nice to make the common case faster -- I can't tell if this is so in the current version.

r=me
Comment 6 mitz 2008-04-18 14:21:45 PDT
Fixed in <http://trac.webkit.org/projects/webkit/changeset/32226>.