LayoutTests:
Reviewed by Hyatt.
- test for http://bugs.webkit.org/show_bug.cgi?id=12726
REGRESSION (r12073): Text wraps in the middle of a word instead of wrapping at the space before the word
* fast/text/break-word-expected.checksum: Added.
* fast/text/break-word-expected.png: Added.
* fast/text/break-word-expected.txt: Added.
* fast/text/break-word.html: Added.
WebCore:
Reviewed by Hyatt.
- fix http://bugs.webkit.org/show_bug.cgi?id=12726
REGRESSION (r12073): Text wraps in the middle of a word instead of wrapping at the space before the word
Test: fast/text/break-word.html
The wrapW variable used to keep track of the width of the characters scanned
so far by adding up the widths of individual characters. Because of the
rounding hack, the total ended up being bigger than the width of the same characters
when measured together as a single run.
The fix is to use wrapW only as an upper bound, and once it overflows the line's width,
fall back on measuring everything from the beginning of the line as one run.
* rendering/bidi.cpp:
(WebCore::RenderBlock::findNextLineBreak): Implemented the above fix, including not measuring
additional single characters once wrapW overflows the line. Also moved the assignment
to breakNBSP out of the loop since it is constant for the entire text object, made breakWords and
midWordBreak update only when they might change, and cleaned up a few things.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@19621 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/rendering/bidi.cpp b/WebCore/rendering/bidi.cpp
index 3a93a32..514fce7 100644
--- a/WebCore/rendering/bidi.cpp
+++ b/WebCore/rendering/bidi.cpp
@@ -2214,13 +2214,13 @@
} else
tmpW += o->width() + o->marginLeft() + o->marginRight() + inlineWidth(o);
} else if (o->isText()) {
- RenderText *t = static_cast<RenderText *>(o);
+ RenderText* t = static_cast<RenderText*>(o);
int strlen = t->textLength();
int len = strlen - pos;
const UChar* str = t->characters();
const Font& f = t->style(m_firstLine)->font();
- // proportional font, needs a bit more work.
+
int lastSpace = pos;
int wordSpacing = o->style()->wordSpacing();
int lastSpaceWordSpacing = 0;
@@ -2229,6 +2229,11 @@
// then assume the start width has been applied.
int wrapW = tmpW + inlineWidth(o, !appliedStartWidth, true);
int nextBreakable = -1;
+ bool breakNBSP = autoWrap && o->style()->nbspMode() == SPACE;
+ // Auto-wrapping text should wrap in the middle of a word only if it could not wrap before the word,
+ // which is only possible if the word is the first thing on the line, that is, if |w| is zero.
+ bool breakWords = o->style()->wordWrap() == BREAK_WORD && ((autoWrap && !w) || currWS == PRE);
+ bool midWordBreak = false;
while (len) {
bool previousCharacterIsSpace = currentCharacterIsSpace;
@@ -2245,9 +2250,9 @@
// Ignore soft hyphens
BidiIterator endMid;
if (pos > 0)
- endMid = BidiIterator(0, o, pos-1);
+ endMid = BidiIterator(0, o, pos - 1);
else
- endMid = BidiIterator(0, previous, previous->isText() ? static_cast<RenderText *>(previous)->textLength() - 1 : 0);
+ endMid = BidiIterator(0, previous, previous->isText() ? static_cast<RenderText*>(previous)->textLength() - 1 : 0);
// Two consecutive soft hyphens. Avoid overlapping midpoints.
if (sNumMidpoints && smidpoints->at(sNumMidpoints - 1).obj == endMid.obj && smidpoints->at(sNumMidpoints - 1).pos > endMid.pos)
sNumMidpoints--;
@@ -2255,14 +2260,14 @@
addMidpoint(endMid);
// Add the width up to but not including the hyphen.
- tmpW += t->width(lastSpace, pos - lastSpace, f, w+tmpW) + lastSpaceWordSpacing;
+ tmpW += t->width(lastSpace, pos - lastSpace, f, w + tmpW) + lastSpaceWordSpacing;
// For wrapping text only, include the hyphen. We need to ensure it will fit
// on the line if it shows when we break.
if (autoWrap)
- tmpW += t->width(pos, 1, f, w+tmpW);
+ tmpW += t->width(pos, 1, f, w + tmpW);
- BidiIterator startMid(0, o, pos+1);
+ BidiIterator startMid(0, o, pos + 1);
addMidpoint(startMid);
}
@@ -2274,18 +2279,17 @@
}
bool applyWordSpacing = false;
- bool breakNBSP = autoWrap && o->style()->nbspMode() == SPACE;
- // FIXME: This check looks suspicious. Why does w have to be 0?
- bool breakWords = o->style()->wordWrap() == BREAK_WORD && ((autoWrap && w == 0) || currWS == PRE);
-
currentCharacterIsWS = currentCharacterIsSpace || (breakNBSP && c == noBreakSpace);
- if (breakWords)
+ if (breakWords && !midWordBreak) {
wrapW += t->width(pos, 1, f, w+wrapW);
- bool midWordBreak = breakWords && (w + wrapW > width);
+ midWordBreak = w + wrapW > width;
+ }
- if (c == '\n' || (currWS != PRE && isBreakable(str, pos, strlen, nextBreakable, breakNBSP)) || midWordBreak) {
+ bool betweenWords = c == '\n' || (currWS != PRE && isBreakable(str, pos, strlen, nextBreakable, breakNBSP));
+
+ if (betweenWords || midWordBreak) {
bool stoppedIgnoringSpaces = false;
if (ignoringSpaces) {
if (!currentCharacterIsSpace) {
@@ -2294,7 +2298,7 @@
ignoringSpaces = false;
lastSpaceWordSpacing = 0;
lastSpace = pos; // e.g., "Foo goo", don't add in any of the ignored spaces.
- BidiIterator startMid ( 0, o, pos );
+ BidiIterator startMid(0, o, pos);
addMidpoint(startMid);
stoppedIgnoringSpaces = true;
} else {
@@ -2314,7 +2318,7 @@
applyWordSpacing = wordSpacing && currentCharacterIsSpace && !previousCharacterIsSpace;
- if (autoWrap && w + tmpW > width && w == 0) {
+ if (autoWrap && w + tmpW > width && !w) {
int fb = nearestFloatBottom(m_height);
int newLineWidth = lineWidth(fb);
// See if |tmpW| will fit on the new line. As long as it does not,
@@ -2392,11 +2396,14 @@
return lBreak;
}
- if (autoWrap) {
+ if (autoWrap && betweenWords) {
w += tmpW;
tmpW = 0;
lBreak.obj = o;
lBreak.pos = pos;
+ // Auto-wrapping text should not wrap in the middle of a word once it has had an
+ // opportunity to break after a word.
+ breakWords = false;
}
if (midWordBreak) {
@@ -2404,6 +2411,7 @@
// adding the end width forces a break.
lBreak.obj = o;
lBreak.pos = pos;
+ midWordBreak &= breakWords;
} else {
lastSpaceWordSpacing = applyWordSpacing ? wordSpacing : 0;
lastSpace = pos;
@@ -2456,11 +2464,9 @@
// IMPORTANT: pos is > length here!
if (!ignoringSpaces)
tmpW += t->width(lastSpace, pos - lastSpace, f, w+tmpW) + lastSpaceWordSpacing;
- if (!appliedStartWidth)
- tmpW += inlineWidth(o, true, false);
- tmpW += inlineWidth(o, false, true);
+ tmpW += inlineWidth(o, !appliedStartWidth, true);
} else
- ASSERT( false );
+ ASSERT_NOT_REACHED();
RenderObject* next = bidiNext(start.block, o, bidi);
bool checkForBreak = autoWrap;
@@ -2527,7 +2533,6 @@
if (!last->isFloatingOrPositioned() && last->isReplaced() && autoWrap &&
(!last->isListMarker() || static_cast<RenderListMarker*>(last)->isInside())) {
- // Go ahead and add in tmpW.
w += tmpW;
tmpW = 0;
lBreak.obj = o;
