RESOLVED FIXED 62743
Support logical line movement in vertical writing mode
https://bugs.webkit.org/show_bug.cgi?id=62743
Summary Support logical line movement in vertical writing mode
Ryosuke Niwa
Reported 2011-06-15 12:03:26 PDT
document.execCommand('move', 'forward'/'backward', 'line') should work in vertical writing writing mode.
Attachments
work in progress (6.44 KB, patch)
2011-06-15 12:35 PDT, Ryosuke Niwa
no flags
work in progress (with tests) (10.57 KB, patch)
2011-06-15 12:37 PDT, Ryosuke Niwa
no flags
fixes the bug (447.28 KB, patch)
2011-06-16 16:49 PDT, Ryosuke Niwa
darin: review+
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-02 (1.55 MB, application/zip)
2011-06-16 17:19 PDT, WebKit Review Bot
no flags
Ryosuke Niwa
Comment 1 2011-06-15 12:35:40 PDT
Created attachment 97343 [details] work in progress It turned out that when moving across p's (-2.html tests), we can't use lineTop for x coordinate for the reasons explained in my patch for the bug 55481.
Ryosuke Niwa
Comment 2 2011-06-15 12:37:45 PDT
Created attachment 97344 [details] work in progress (with tests) Oops, forgot to add tests.
Ryosuke Niwa
Comment 3 2011-06-16 16:49:09 PDT
Created attachment 97520 [details] fixes the bug
Ryosuke Niwa
Comment 4 2011-06-16 16:52:51 PDT
This bug is about fixing modify('move', 'forward'/'backward', 'line'), not about arrow keys navigations. That's tracked by the bug 62833 and blocked by this bug. The code change itself is pretty trivial; the patch is big because I've added many tests and renamed some functions.
Darin Adler
Comment 5 2011-06-16 17:16:17 PDT
Comment on attachment 97520 [details] fixes the bug It would make this patch easier to read if the renaming was done in a separate preparation step. The part of this code I am not sure about is absoluteLineDirectionPointToLocalPointInBlock.
WebKit Review Bot
Comment 6 2011-06-16 17:19:39 PDT
Comment on attachment 97520 [details] fixes the bug Attachment 97520 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8879373 New failing tests: editing/selection/vertical-rl-ltr-extend-line-backward-p.html editing/selection/vertical-rl-rtl-extend-line-forward-p.html editing/selection/vertical-rl-ltr-extend-line-forward-br.html editing/selection/vertical-rl-rtl-extend-line-backward-br.html editing/selection/vertical-rl-rtl-extend-line-forward-br.html editing/selection/vertical-rl-ltr-extend-line-forward-wrap.html editing/selection/vertical-rl-rtl-extend-line-backward-p.html editing/selection/vertical-rl-ltr-extend-line-forward-p.html editing/selection/vertical-lr-ltr-extend-line-backward-br.html editing/selection/vertical-lr-ltr-extend-line-forward-br.html editing/selection/vertical-rl-ltr-extend-line-backward-br.html editing/selection/vertical-rl-ltr-extend-line-backward-wrap.html
WebKit Review Bot
Comment 7 2011-06-16 17:19:45 PDT
Created attachment 97525 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Ryosuke Niwa
Comment 8 2011-06-16 17:21:36 PDT
Thanks for a review! (In reply to comment #5) > (From update of attachment 97520 [details]) > It would make this patch easier to read if the renaming was done in a separate preparation step. The part of this code I am not sure about is absoluteLineDirectionPointToLocalPointInBlock. Yeah, I don't like that function either. I rewrote it a couple of times to get it feel right but it's still ugly :( I'm more than happy to do some refactoring in follow up patches.
Ryosuke Niwa
Comment 9 2011-06-16 17:23:22 PDT
Of course, these tests require platform-specific results so I'll stick around on #webkit and rebaseline as needed.
Ryosuke Niwa
Comment 10 2011-06-16 17:25:48 PDT
Note You need to log in before you can comment on or make changes to this bug.