Bug 13736 – REGRESSION (r19811): Using the down arrow in a textarea gets "stuck" at the end of a wrapped line

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[David Kilzer (:ddkilzer)]

* SUMMARY
After typing text into a textarea, scrolling down using the down arrow sometimes gets stuck at the end of a wrapped line.

* STEPS TO REPRODUCE
1. Type text into a (LTR) textarea.
2. Move cursor to the top-left corner so that using the down-arrow moves the cursor only vertically (to the beginning of each line).
3. Start using the down arrow on the typed text.

* EXPECTED RESULTS
The down arrow should move the cursor all the way to the bottom of the textarea, and the cursor should stay on the far left side of every line.

* ACTUAL RESULTS
Occasionally, hitting the down arrow will move the cursor to the end of the current line (instead of to the beginning of the next line), and additional down arrow key presses will not move it any further.

* REGRESSION
This is a regression from shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.9 (8P135).  This happened on a local debug build of WebKit r21489 with the aforementioned software.

* NOTES
I can't reproduce this on demand.  I believe it has something to do with how the text is edited in the textarea, creating some kind of internal representation that misbehaves when using the down arrow.

[David Kilzer (:ddkilzer)]

I most recently saw this while typing in the "Comment (on this bug):" field on the edit attachment page for Bug 13732:  http://bugs.webkit.org/attachment.cgi?id=14568&action=edit

[Terry Riegel]

Similarly, navigating a textarea set to no wrap with the arrows places/hides the cursor with the right scrollbar and the bottom scrollbar.

To duplicate add enough lines to create a right scrollbar, then midway through create a line long enough to create a bottom scrollbar. (by bottom scrollbar I mean positioned at the bottom of the texarea for scrolling left and right, and by right scrollbar I mean positioned at the right of the textarea)

Then using just the arrow keys navigate to the line below your long line. Hit the left arrow, This should place your cursor at the end of the really long line. It does but it fails to scroll completely so it can be seen.

The offset is the width of the scrollbars.

[Terry Riegel]

Similarly, navigating a textarea set to no wrap with the arrows places/hides the cursor with the right scrollbar and the bottom scrollbar.

To duplicate add enough lines to create a right scrollbar, then midway through create a line long enough to create a bottom scrollbar. (by bottom scrollbar I mean positioned at the bottom of the texarea for scrolling left and right, and by right scrollbar I mean positioned at the right of the textarea)

Then using just the arrow keys navigate to the line below your long line. Hit the left arrow, This should place your cursor at the end of the really long line. It does but it fails to scroll completely so it can be seen.

The offset is the width of the scrollbars.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> Similarly, navigating a textarea set to no wrap with the arrows places/hides
> the cursor with the right scrollbar and the bottom scrollbar.

Terry, please file a new bug about this issue.  Thanks!

[David Kilzer (:ddkilzer)]

* STEPS TO REPRODUCE
1. Open this bug (or any Bugzilla bug on bugs.webkit.org).

2. Hit Cmd-L to select URL for the current bugs.webkit.org page you're on, then hit Cmd-C to copy it to the pasteboard.

3. Click in "Additional Comments:" textarea to give it focus.

4. Type the following line of text (without double quotes) but DO NOT hit Enter at the end; it should be on a single line; note the space at the end of the line:

"2. Follow the Windows instructions on this page to check out the source code: "

5. Hit Cmd-V to paste the URL (which should wrap to the next line).

6. Hit Enter to create a new line and move the caret to a blank line.

7. Hit the up arrow key twice to move the caret to the top left corner of the textarea.

8. Hit the down arrow twice.

* EXPECTED RESULTS
The caret should be on the third (blank) line of the textarea.

* ACTUAL RESULTS
The caret gets "stuck" at the end of the first line.

* NOTES
It probably doesn't matter what text is typed or what is pasted as long as a word wrap occurs when the paste happens.

[David Kilzer (:ddkilzer)]

<rdar://problem/5347931>

[David Kilzer (:ddkilzer)]

Using my new autospade script (Bug 15002):

Reproduced bug: r19818
Did NOT reproduce bug: r19809

[Justin Garcia]

(In reply to comment #7)
> Using my new autospade script (Bug 15002):
> 
> Reproduced bug: r19818
> Did NOT reproduce bug: r19809

Probably regressed in r19811.

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> Probably regressed in r19811.

Verified with internal autospade:

Works: r19810  Fails: r19811

[Eric Seidel (no email)]

Wow.  I was just able to reproduce it as I was typing a comment about how I couldn't... so strange.  I'm on TOT (r26859).

[Eric Seidel (no email)]

I just reproduced this w/o any pasting.  I was moving up and down in a scrolled text area on Facebook.

[David Kilzer (:ddkilzer)]

See also Bug 19465, although that was a slightly different issue.

[Alp Toker]

Issue also shows up with the caret browsing patches applied (bug #16135).

[Justin Garcia]

I believe that the xposfor... code is correct.  What's we're seeing is a symptom of:

https://bugs.webkit.org/show_bug.cgi?id=9535

[Beth Dakin]

Created attachment 25644 [details]
Patch

Comment on attachment 25644 [details]
Patch

> +    for (RenderObject* next = renderer->nextInPreOrder((RenderObject*)container); 

You should not need to cast container, which is a RenderBlock*, to RenderObject*.

I wonder if this for loop can be made clearer by rewriting it as a while loop and only one copy of "next = renderer->nextInPreOrder(container)".

Can you try making the tests dump as text? I think you can get the selection in JavaScript and test if it is in the node and offset you expect it to be and output PASS/FAIL accordingly.

[Beth Dakin]

Created attachment 25664 [details]
Newer Patch

This patch addresses Dan's first two comments.

The last comment about the layout tests made me have a realization. I remember when Darin and I were debugging this together, we realized that the selection was actually correct before the fix. The caret was just painting in the wrong place. So actually, these tests won't catch the bug as they are written in dump render tree (except with pixel tests of course). And changing the tests as Dan suggests definitely would not fix the bug. I think the best solution is to keep these tests as they are, and unfortunately, they only mean something when the pixel tests are run.

Comment on attachment 25664 [details]
Newer Patch

Yup, asking about turning the tests into text-only tests was silly. r=me

[Beth Dakin]

Oh, it wasn't too silly. I wrote the patch, and it took me a bit of testing and debugging to remember why getting the selection wouldn't work.

Fixed with r38898.

[David Kilzer (:ddkilzer)]

It's too bad the test case can't report the row/column of the insertion bar (as text) when it's done running.  Did that differ before and after the fix?

[Alejandro G. Castro]

Apparently this patch is causing problems with the caret browsing, the issue in the bug 25676 does not happen if we avoid the call to the searchAheadForBetterMatch.

[Alejandro G. Castro]

(In reply to comment #21)
> Apparently this patch is causing problems with the caret browsing, the issue in
> the bug 25676 does not happen if we avoid the call to the
> searchAheadForBetterMatch.

Actually if we use the test uploaded in the patch (wrapped-line-caret-1.html) we can check that if we move the caret to the last position (after the last x) and we move up we can not reach the first line. I would say the patch fixes the problem in one situation but breaks the opposite use case, causing also a problem when we have inlines and we move with the cursor with the caret browsing, like in the bug 25676.