Bug 54598

Summary: Trailing tabs in a textarea become unselectable under certain conditions
Product: WebKit Reporter: Nick Santos <nicksantos>
Component: Layout and RenderingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, bdakin, darin, dglazkov, enrica, eric, hyatt, justin.garcia, komoroske, leviw, mitz, ojan, peterbraden, rniwa, rolandsteiner, webkit.review.bot, xji
Priority: P2 Keywords: GoogleBug, HasReduction
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 57745, 61483    
Attachments:
Description Flags
Reduced test case
none
work in progress
none
fixes the bug
none
test case without contenteditable attribute
none
fixes the bug
hyatt: review+, webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-03 none

Nick Santos
Reported 2011-02-16 16:06:21 PST
Created attachment 82716 [details] Reduced test case Under certain circumstances, I can get a textarea into a state where tabs at the end of the textarea can't be selected. It happens with certain values and not others, and depends a lot on the contents of the textarea. So I've just attached a reduced test case that demonstrates the issue. The test fails on Chrome 9 Linux and Win (Webkit 534.13), and Safari/Win 5.03 (Webkit 533.19). You should get the alert() message: Expected selectionEnd : 45 Actual selectionEnd : 44
Attachments
Reduced test case (765 bytes, text/html)
2011-02-16 16:06 PST, Nick Santos
no flags
work in progress (291.40 KB, patch)
2011-04-04 07:48 PDT, Ryosuke Niwa
no flags
fixes the bug (209.26 KB, patch)
2011-04-04 10:55 PDT, Ryosuke Niwa
no flags
test case without contenteditable attribute (1.32 KB, text/html)
2011-06-06 18:39 PDT, Ryosuke Niwa
no flags
fixes the bug (39.74 KB, patch)
2011-06-13 17:31 PDT, Ryosuke Niwa
hyatt: review+
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-03 (1.40 MB, application/zip)
2011-06-13 17:54 PDT, WebKit Review Bot
no flags
Ojan Vafai
Comment 1 2011-02-16 20:31:44 PST
Nick, this is causing a bug in Google Spreadsheets, right? In either case, it's clearly a bug.
Nick Santos
Comment 2 2011-02-16 20:38:19 PST
yes. We care about it because when people copy-and-paste from spreadsheets, chopping off tabs at the end of Tab-Separated-Values is really bad. It drastically changes which cells are changed. The bug only reproduces when the textarea has certain values, so our users see non-deterministic behavior.
Ojan Vafai
Comment 3 2011-03-10 00:14:23 PST
Ryosuke, did the fix for bug 56061 also fix this?
Ryosuke Niwa
Comment 4 2011-03-10 11:17:07 PST
(In reply to comment #3) > Ryosuke, did the fix for bug 56061 also fix this? Unfortunately not. From what I can tell, this seems to be whitespace balancing or renderer issue.
Ryosuke Niwa
Comment 5 2011-03-25 02:24:29 PDT
It seems like this is a bug in the renderer. When there's a vertical scroll bar, the length is reported wrongfully but the length is reported correctly when textarea is large enough to not have a scrollbar. This bug affects Google Docs and we'd like it to be fixed ASAP.
Ryosuke Niwa
Comment 6 2011-04-04 07:48:39 PDT
Created attachment 88057 [details] work in progress Here's my work in progress patch. Will write change log entires later today. I'm very excited to fix this bug!
Ryosuke Niwa
Comment 7 2011-04-04 10:55:22 PDT
Created attachment 88080 [details] fixes the bug
Ryosuke Niwa
Comment 8 2011-04-04 10:56:50 PDT
This patch also fixes the bug 57745. I'll add a test for the bug 57745 once this patch is landed.
Ryosuke Niwa
Comment 9 2011-04-05 04:20:38 PDT
Comment on attachment 88080 [details] fixes the bug This patch is wrong. It'll show the space only if the line wasn't empty so you end up with a weird behavior where the trailing tab shows up on the last line but typing any other character after the tab collapses the tab again. Need to do some reading of CSS2.1 spec.
Ryosuke Niwa
Comment 10 2011-04-05 11:58:44 PDT
Upon further investigation, I figured out that the bug reproduces iff word-wrap is set to break-word. So the problem doesn't reproduce in the following div: <div style="width: 7ex; font-size: 1em; white-space: pre-wrap; -webkit-line-break: normal; word-wrap: normal;" contenteditable></div> but reproduces in: <div style="width: 7ex; font-size: 1em; white-space: pre-wrap; -webkit-line-break: normal;" contenteditable></div> because contenteditable element automatically gets word-wrap: break-word.
Ryosuke Niwa
Comment 11 2011-05-25 23:36:51 PDT
The bug 61484 might be a duplicate of this bug.
Ryosuke Niwa
Comment 12 2011-05-26 11:34:19 PDT
*** Bug 61483 has been marked as a duplicate of this bug. ***
Ryosuke Niwa
Comment 13 2011-06-06 14:40:36 PDT
Ryosuke Niwa
Comment 14 2011-06-06 16:08:57 PDT
I admit my defeat. I don't know enough about line layout to be able to fix this bug in any foreseeable future.
Ryosuke Niwa
Comment 15 2011-06-06 18:39:04 PDT
Created attachment 96170 [details] test case without contenteditable attribute
Eric Seidel (no email)
Comment 16 2011-06-07 17:00:52 PDT
Are we sure this isn't a regression?
Ryosuke Niwa
Comment 17 2011-06-07 17:52:27 PDT
It turned out that the spec specifies whitespace wordwrap behavior: http://www.w3.org/TR/CSS2/text.html#white-space-model
Ryosuke Niwa
Comment 18 2011-06-07 19:13:07 PDT
16.6.1: 2. If 'white-space' is set to 'pre' or 'pre-wrap', any sequence of spaces (U+0020) unbroken by an element boundary is treated as a sequence of non-breaking spaces. However, for 'pre-wrap', a line breaking opportunity exists at the end of the sequence. ... 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. ... 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. It's the combination of these 3 paragraphs that get us into trouble. If we never collapsed tabs or spaces at the end of a line with 'white-space' set to 'pre-wrap', this bug will go away.
Ryosuke Niwa
Comment 19 2011-06-13 16:57:23 PDT
(In reply to comment #18) > 16.6.1: > 2. If 'white-space' is set to 'pre' or 'pre-wrap', any sequence of spaces (U+0020) unbroken by an element boundary is treated as a sequence of non-breaking spaces. However, for 'pre-wrap', a line breaking opportunity exists at the end of the sequence. > > ... > 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. > ... > 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. > > It's the combination of these 3 paragraphs that get us into trouble. If we never collapsed tabs or spaces at the end of a line with 'white-space' set to 'pre-wrap', this bug will go away. Uh-huh! We're violating point 1 by applying point 4 to the leading whitespace as well. We can fix this bug by NOT collapsing leading whitespace when 'white-space' is set to 'pre-wrap'.
Ryosuke Niwa
Comment 20 2011-06-13 17:31:47 PDT
Created attachment 97045 [details] fixes the bug
WebKit Review Bot
Comment 21 2011-06-13 17:54:43 PDT
Comment on attachment 97045 [details] fixes the bug Attachment 97045 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8833678 New failing tests: fast/forms/basic-textareas-quirks.html fast/forms/basic-textareas.html
WebKit Review Bot
Comment 22 2011-06-13 17:54:49 PDT
Created attachment 97050 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Dave Hyatt
Comment 23 2011-06-14 13:38:41 PDT
Comment on attachment 97045 [details] fixes the bug r=me
Ryosuke Niwa
Comment 24 2011-06-14 17:19:44 PDT
Ryosuke Niwa
Comment 25 2011-06-14 17:20:04 PDT
(In reply to comment #23) > (From update of attachment 97045 [details]) > r=me Thanks a lot for the review!
Ryosuke Niwa
Comment 26 2011-06-14 17:20:53 PDT
Note You need to log in before you can comment on or make changes to this bug.