Bug 13438 - Run rounding makes word-break:break-all/word not functional
Summary: Run rounding makes word-break:break-all/word not functional
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Dave Hyatt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2007-04-21 14:32 PDT by Dave Hyatt
Modified: 2007-07-13 16:53 PDT (History)
1 user (show)

See Also:


Attachments
Avoid rounding errors with word-break:break-{all,word} (70.43 KB, patch)
2007-07-12 05:58 PDT, mitz
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2007-04-21 14:32:08 PDT
<div style="position:absolute;border:2px solid black;word-break:break-all;font-family:'Lucida Grande'">Maxwidth is wrong when word-break all is set.  More and more extra pixels will creep in.</div>

<div style="position:absolute;top:40px;border:2px solid black;font-family:'Lucida Grande'">Maxwidth is wrong when word-break all is set.  More and more extra pixels will creep in.</div>

The problem is that breaking on every character causes integer rounding to kick in for each character because of run rounding.
Comment 1 Dave Hyatt 2007-04-21 14:32:37 PDT
P1.  Necessary for the feature to work at all.
Comment 2 Dave Hyatt 2007-04-21 14:35:06 PDT
For break-all only, we'll also end up breaking too early because rounding errors will accumulate during findNextLineBreak.
Comment 3 mitz 2007-04-21 23:42:35 PDT
(In reply to comment #2)
> For break-all only, we'll also end up breaking too early because rounding
> errors will accumulate during findNextLineBreak.
> 

For word-wrap:break-word, findNextLineBreak does the right thing: it uses the accumulated width ('wrapW') as an upper bound on the actual width. Once the bound is greater than 'width', it starts checking the true width. Looks like the same logic should apply to break-all. In fact, this

                if (breakAll || (breakWords && !midWordBreak)) {
                    wrapW += t->width(pos, 1, f, w + wrapW);
                    midWordBreak = w + wrapW > width;
                }

should have taken care of it, making the 'breakAll' check in

                if (betweenWords || midWordBreak || breakAll) {

redundant if you also changed

                        midWordBreak &= breakWords;

to say 'breakWords || breakAll'. Actually, I think the first condition could be

                if ((breakAll || breakWords) && !midWordBreak) {
Comment 4 Darin Adler 2007-04-23 08:40:09 PDT
<rdar://problem/5153030>
Comment 5 mitz 2007-07-12 05:58:53 PDT
Created attachment 15485 [details]
Avoid rounding errors with word-break:break-{all,word}

No layout test regressions. Includes tests for max width and line breaking.
Comment 6 Dave Hyatt 2007-07-13 15:11:39 PDT
Comment on attachment 15485 [details]
Avoid rounding errors with word-break:break-{all,word}

r=me
Comment 7 mitz 2007-07-13 16:53:09 PDT
Landed by Sam in r24278.