[webkit-reviews] review granted: [Bug 17921] Extra white space at
the end of right-aligned or justified text with
-webkit-line-break: after-white-space : [Attachment 20675]
Correct trailing whitespace behavior
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 18 13:38:38 PDT 2008
Darin Adler <darin at apple.com> has granted mitz at webkit.org's request for review:
Bug 17921: Extra white space at the end of right-aligned or justified text with
-webkit-line-break: after-white-space
http://bugs.webkit.org/show_bug.cgi?id=17921
Attachment 20675: Correct trailing whitespace behavior
http://bugs.webkit.org/attachment.cgi?id=20675&action=edit
------- Additional Comments from Darin Adler <darin at apple.com>
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
More information about the webkit-reviews
mailing list