Bug 13736

Summary: REGRESSION (r19811): Using the down arrow in a textarea gets "stuck" at the end of a wrapped line
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, alp, ap, bdakin, bugs, harrison, jonathon, justin.garcia, mitz
Priority: P1 Keywords: InRadar, Regression
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Patch
none
Newer Patch mitz: review+

David Kilzer (:ddkilzer)
Reported 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.
Attachments
Patch (9.75 KB, patch)
2008-12-01 17:01 PST, Beth Dakin 		no flags
DetailsFormatted DiffDiff
Newer Patch (9.96 KB, patch)
2008-12-01 22:20 PST, Beth Dakin 		mitz: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 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
Terry Riegel
Comment 2 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.
Terry Riegel
Comment 3 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.
David Kilzer (:ddkilzer)
Comment 4 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!
David Kilzer (:ddkilzer)
Comment 5 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.
David Kilzer (:ddkilzer)
Comment 6 2007-07-19 13:50:28 PDT
<rdar://problem/5347931>
David Kilzer (:ddkilzer)
Comment 7 2007-08-18 18:04:01 PDT
Using my new autospade script (Bug 15002): Reproduced bug: r19818 Did NOT reproduce bug: r19809
Justin Garcia
Comment 8 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.
David Kilzer (:ddkilzer)
Comment 9 2007-09-02 21:09:47 PDT
(In reply to comment #8) > Probably regressed in r19811. Verified with internal autospade: Works: r19810 Fails: r19811
Eric Seidel (no email)
Comment 10 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).
Eric Seidel (no email)
Comment 11 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.
David Kilzer (:ddkilzer)
Comment 12 2008-06-23 14:01:03 PDT
See also Bug 19465, although that was a slightly different issue.
Alp Toker
Comment 13 2008-10-18 14:35:47 PDT
Issue also shows up with the caret browsing patches applied (bug #16135).
Justin Garcia
Comment 14 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
Beth Dakin
Comment 15 2008-12-01 17:01:08 PST
Created attachment 25644 [details] Patch
mitz
Comment 16 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.
Beth Dakin
Comment 17 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.
mitz
Comment 18 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
Beth Dakin
Comment 19 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.
David Kilzer (:ddkilzer)
Comment 20 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?
Alejandro G. Castro
Comment 21 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.
Alejandro G. Castro
Comment 22 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.
Note You need to log in before you can comment on or make changes to this bug.