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:

  1. When white-space: pre-wrap and word-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.
  2. 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,

  1. If a space (U+0020) at the beginning of a line has white-space set to normal, nowrap, or pre-line, it is removed.
  2. 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.
  3. If a space (U+0020) at the end of a line has white-space set to normal, nowrap, or pre-line, it is also removed.
  4. If spaces (U+0020) or tabs (U+0009) at the end of a line have white-space set to pre-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.