Bug 70757 - Moving to the start of line should not place the caret outside of the table
Summary: Moving to the start of line should not place the caret outside of the table
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on: 70755
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-24 13:49 PDT by Ryosuke Niwa
Modified: 2011-10-25 16:17 PDT (History)
8 users (show)

See Also:


Attachments
test (1.57 KB, text/html)
2011-10-24 14:30 PDT, Ryosuke Niwa
no flags Details
fixes the bug (5.38 KB, patch)
2011-10-25 11:40 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-10-24 13:49:00 PDT
When moving to the start of a line (command + left arrow key on Mac; Home on Windows) inside the first table cell, the caret is placed before of the table but this behavior is inconsistent with Firefox, TextEdit, Microsoft Word, which all place the caret in the first position inside the table cell.
Comment 1 Ojan Vafai 2011-10-24 14:16:30 PDT
Do you have a testcase we can play with?
Comment 2 Ryosuke Niwa 2011-10-24 14:30:15 PDT
Created attachment 112247 [details]
test
Comment 3 Julie Parent 2011-10-24 15:00:22 PDT
Does this imply that there is no keyboard navigable way to get out of the table?  What about the reciprocal behavior of moving to the end of the line?
Comment 4 Ryosuke Niwa 2011-10-24 15:04:34 PDT
(In reply to comment #3)
> Does this imply that there is no keyboard navigable way to get out of the table?  What about the reciprocal behavior of moving to the end of the line?

There is. You just need to move using arrow keys without any modifiers, which is what all other browsers and apps do.

Our current behavior is broken and inconsistent. There's no way for users to move to the beginning of a table cell.

Also, moving to the end of a line at the end last table cell doesn't move the caret to a position after the table :( So we're inconsistent with our own behavior.
Comment 5 Ryosuke Niwa 2011-10-24 15:10:10 PDT
One clarification: neither Firefox nor Internet Explorer lets you move the caret before or after the table in the test case I attached. WebKit provides an extra position before/after the table.
Comment 6 Ojan Vafai 2011-10-24 15:22:29 PDT
(In reply to comment #5)
> One clarification: neither Firefox nor Internet Explorer lets you move the caret before or after the table in the test case I attached. WebKit provides an extra position before/after the table.

That's a bug IMO. Firefox/IE should be fixed.
Comment 7 Ryosuke Niwa 2011-10-25 11:40:24 PDT
Created attachment 112366 [details]
fixes the bug
Comment 8 Chang Shu 2011-10-25 12:03:15 PDT
Maybe I missed the previous conversations. I found the following two commits that introduced the function and a fix to that:
http://trac.webkit.org/changeset/25586
http://trac.webkit.org/changeset/63918

Would this code change break the tests in the commits above? Or shall we update their expectations?
Comment 9 Ryosuke Niwa 2011-10-25 12:19:48 PDT
(In reply to comment #8)
> Maybe I missed the previous conversations. I found the following two commits that introduced the function and a fix to that:
> http://trac.webkit.org/changeset/25586
> http://trac.webkit.org/changeset/63918
> 
> Would this code change break the tests in the commits above? Or shall we update their expectations?

All tests still pass.
Comment 10 Chang Shu 2011-10-25 15:06:45 PDT
Comment on attachment 112366 [details]
fixes the bug

The code change matches the new behavior. r=me.
Comment 11 Ryosuke Niwa 2011-10-25 15:07:23 PDT
Thanks for the review.
Comment 12 WebKit Review Bot 2011-10-25 16:17:08 PDT
Comment on attachment 112366 [details]
fixes the bug

Clearing flags on attachment: 112366

Committed r98408: <http://trac.webkit.org/changeset/98408>
Comment 13 WebKit Review Bot 2011-10-25 16:17:13 PDT
All reviewed patches have been landed.  Closing bug.