Bug 13736 - REGRESSION (r19811): Using the down arrow in a textarea gets "stuck" at the end of a wrapped line
Summary: REGRESSION (r19811): Using the down arrow in a textarea gets "stuck" at the e...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-05-15 16:09 PDT by David Kilzer (:ddkilzer)
Modified: 2009-10-12 14:53 PDT (History)
9 users (show)

See Also:


Attachments
Patch (9.75 KB, patch)
2008-12-01 17:01 PST, Beth Dakin
no flags Details | Formatted Diff | Diff
Newer Patch (9.96 KB, patch)
2008-12-01 22:20 PST, Beth Dakin
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2007-05-15 16:09:41 PDT
* 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.
Comment 1 David Kilzer (:ddkilzer) 2007-05-15 16:11:12 PDT
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

Comment 2 Terry Riegel 2007-06-20 06:29:04 PDT
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.
Comment 3 Terry Riegel 2007-06-20 06:30:16 PDT
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.
Comment 4 David Kilzer (:ddkilzer) 2007-06-20 08:57:00 PDT
(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!
Comment 5 David Kilzer (:ddkilzer) 2007-07-19 10:11:47 PDT
* 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.

Comment 6 David Kilzer (:ddkilzer) 2007-07-19 13:50:28 PDT
<rdar://problem/5347931>
Comment 7 David Kilzer (:ddkilzer) 2007-08-18 18:04:01 PDT
Using my new autospade script (Bug 15002):

Reproduced bug: r19818
Did NOT reproduce bug: r19809

Comment 8 Justin Garcia 2007-08-20 11:42:51 PDT
(In reply to comment #7)
> Using my new autospade script (Bug 15002):
> 
> Reproduced bug: r19818
> Did NOT reproduce bug: r19809

Probably regressed in r19811.
Comment 9 David Kilzer (:ddkilzer) 2007-09-02 21:09:47 PDT
(In reply to comment #8)
> Probably regressed in r19811.

Verified with internal autospade:

Works: r19810  Fails: r19811

Comment 10 Eric Seidel (no email) 2007-10-21 21:26:54 PDT
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).
Comment 11 Eric Seidel (no email) 2007-10-23 16:13:10 PDT
I just reproduced this w/o any pasting.  I was moving up and down in a scrolled text area on Facebook.
Comment 12 David Kilzer (:ddkilzer) 2008-06-23 14:01:03 PDT
See also Bug 19465, although that was a slightly different issue.
Comment 13 Alp Toker 2008-10-18 14:35:47 PDT
Issue also shows up with the caret browsing patches applied (bug #16135).
Comment 14 Justin Garcia 2008-11-21 17:54:05 PST
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
Comment 15 Beth Dakin 2008-12-01 17:01:08 PST
Created attachment 25644 [details]
Patch
Comment 16 mitz 2008-12-01 19:34:31 PST
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.
Comment 17 Beth Dakin 2008-12-01 22:20:59 PST
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 18 mitz 2008-12-01 22:27:43 PST
Comment on attachment 25664 [details]
Newer Patch

Yup, asking about turning the tests into text-only tests was silly. r=me
Comment 19 Beth Dakin 2008-12-01 23:25:35 PST
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.
Comment 20 David Kilzer (:ddkilzer) 2008-12-02 02:52:56 PST
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?

Comment 21 Alejandro G. Castro 2009-10-12 12:16:27 PDT
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.
Comment 22 Alejandro G. Castro 2009-10-12 14:53:31 PDT
(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.