WebKit Bug 54598: "Trailing tabs in a textarea become unselectable under certain conditions"
Last week, I landed the WebKit changeset r88883 to fix the bug 54598 -- "Trailing tabs in a textarea become unselectable under certain conditions". This bug was a quite nasty as it also caused the bug 61483 -- "Textarea will not accept space characters at end of line".
When fixing a WebKit bug, the first thing you do is to make a simple reduction and
figure out the exact condition in which this bug manifests.
After stepping through FindNextLineBreak
through debugger,
I figured out that the bug reproduces if and only if the text run has white-space: pre-wrap
and word-wrap: break-word
and the trailing whitespace does not fit in the last line
(See the reduced test case).
In fact, textarea was nothing to do with this bug.
Once I found a reduction, the next step is to figure out the root cause of the bug. After days of debugging and cleaning up code, I figured that the following was happening:
- When
white-space: pre-wrap
andword-wrap: break-word
and present and the last non-whitespace character does not fit and the trailing whitespace, WebKit breaks the line before the trailing whitespace. - However, WebKit skips leading whitespace in a line wrapped from the previous line
(i.e. the previous line did not end with
\n
in new-line preserving context,br
element, or any block element).
The simplest solution seemed that we need to either not break a line before the trailing whitespace or not skip the leading whitespace in the next line.
I tried both, and it seemed that the latter approach seemed easier and match Firefox's behavior better.
Skipping leading or trailing whitespace is implemented by skipLeadingWhitespace
and skipTrailingWhitespace
in WebKit,
and they both call requiresLineBox
to determine whether a text run should generate
an inline text box or not (an inline text box is an object created for any visible text run in WebKit).
requiresLineBox
in turn calls shouldCollapseWhiteSpace:
static inline bool shouldCollapseWhiteSpace(
const RenderStyle* style, bool isLineEmpty, bool previousLineBrokeCleanly)
{
return style->collapseWhiteSpace()
|| (style->whiteSpace() == PRE_WRAP && (!isLineEmpty || !previousLineBrokeCleanly));
}
What we should be paying attention is the second clause that applies to text runs
with style white-space: pre-wrap
.
We collapse whitespace if the current line is not empty or the previous line did not break cleanly
(i.e. previous did not wrap).
In order to fix the bug 54598), the minimum change we can make is
to not collapse whitespace when the previous line did not break cleanly because when a line is not empty,
the leading whitespace isn't a trailing whitespace and this bug does not reproduce.
So that's what I did.
I was not comfortable with the fact we're doing different things for leading and trailing whitespace
but I couldn't figure out a better way since CSS Text Level 3 module
didn't seem to specify this specific behavior.
To my dismay, I soon found a weird behavior exhibit by WebKit with my patch applied.
Because new shouldCollapseWhiteSpace
still allows the leading whitespace to collapse if the line is not empty,
when a user types any non-whitespace character, the leading whitespace suddenly disappeared.
e.g. if "a " was in a block with both white-space: pre-wrap
and word-wrap: break-word
set
and the line wrapped between "a" and " ",
then " " will be shown on the second line for a moment
until when a user types another non-whitespace such as "b".
In this case because the second line is not empty,
the leading whitespace (space between "a" and "b") is collapsed.
The end result looked as if whitespace suddenly disappeared.
Since I'm not a rendering engine expert, this was bad.
I couldn't figure out the correct solution, and left the bug with a comment
I admit my defeat. I don't know enough about line layout to be able to fix this bug in any foreseeable future.
The things took an interesting turn a day after I posted that comment. I had a conversation with eseidel about the bug, and we started looking into the guts of the issue. He quickly pointed out that this behavior should be well-specified in CSS spec. It turned out that this behavior was well-specified in CSS 2.1 section 16.6.1. Of course! The new CSS3 spec doesn't need to re-specify behavior already well specified by the last spec. The section 16.6.1 had a very nice section that explained how whitespace is handled as lines flows are resolved:
As each line is laid out,
- If a space (
U+0020
) at the beginning of a line haswhite-space
set tonormal
,nowrap
, orpre-line
, it is removed.- All tabs (
U+0009
) are rendered as a horizontal shift that lines up the start edge of the next glyph with the next tab stop. Tab stops occur at points that are multiples of 8 times the width of a space (U+0020
) rendered in the block's font from the block's starting content edge.- If a space (
U+0020
) at the end of a line haswhite-space
set tonormal
,nowrap
, orpre-line
, it is also removed.- If spaces (
U+0020
) or tabs (U+0009
) at the end of a line havewhite-space
set topre-wrap
, UAs may visually collapse them.Floated and absolutely-positioned elements do not introduce a line breaking opportunity.
Now look carefully.
Points 1 and 3 states whitespace should be removed when white-space
is set to normal
,
nowrap
or pre-line
at the beginning or the end of a line.
However, it does not state that whitespace can be removed or collapsed when white-space
is set to pre-wrap
.
Point 4 justifies this behavior at the end of a line but nothing allows WebKit
to skip whitespace at the beginning of a line.
There you have it!
The solution was simple.
We should have never skipped (i.e. collapsed) leading whitespace
when white-space
is set to pre-wrap
in wrapped lines.
After figuring out that part, the fix was easy.
I just made shouldCollapseWhiteSpace
return false when white-space
is pre-wrap
and whitespace is not at the end of a line.