visual word movement relies isWordTextBreak. isWordTextBreak returns true if the word break iterator moving across a word, and it returns false if the word break iterator moving across a punctuation/space. In platforms using ICU, isWordTextBreak uses ICU break iterator functionality.
I cannot find isWordTextBreak anywhere in the WebKit source code. Is this something you're working on now? For the record GTK+ uses ICU by default, so there shouldn't be much in the way of implementation.
(In reply to comment #1) > I cannot find isWordTextBreak anywhere in the WebKit source code. Is this something you're working on now? For the record GTK+ uses ICU by default, so there shouldn't be much in the way of implementation. It is added in r110965 under Source/WebCore/platform/text/TextBreakIterator.h. If you look at its implementation under TextBreakIteratorICU.cpp, basically, it is checking the current word break iterator just pass over a word. For example, "abc def", both positions "abc| |def" are word break positions, but isWordTextBreak returns true for "abc| def" since the break iterator just pass over "abc". And it returns false for "abc |def" since the break iterator just pass over a space. If GTK+ uses ICU, that should be easy to implemented.
(In reply to comment #2) > If GTK+ uses ICU, that should be easy to implemented. Look like this just needs to be implemented for the GLib unicode backend, which we don't have complete support for anyway. By default GTK+ uses Source/WebCore/platform/text/TextBreakIteratorICU.cpp.
(In reply to comment #3) > (In reply to comment #2) > > > If GTK+ uses ICU, that should be easy to implemented. > > Look like this just needs to be implemented for the GLib unicode backend, which we don't have complete support for anyway. By default GTK+ uses Source/WebCore/platform/text/TextBreakIteratorICU.cpp. Ah, then we can turn it on for GTK. I thought GTK uses TextBreakIteratorGtk.cpp.
(In reply to comment #2) > It is added in r110965 > under Source/WebCore/platform/text/TextBreakIterator.h. > > If you look at its implementation under TextBreakIteratorICU.cpp, > basically, it is checking the current word break iterator just pass over a word. > For example, "abc def", both positions "abc| |def" are word break positions, > but isWordTextBreak returns true for "abc| def" since the break iterator just pass over "abc". And it returns false for "abc |def" since the break iterator just pass over a space. > > If GTK+ uses ICU, that should be easy to implemented. I've tried running the Skipped tests, and all but one run without problems in webkitgtk, as it uses ICU backend. The only test failing is editing/selection/move-by-word-visually-multi-line, with this kind of failure: Test 1, LTR: Move right by one word -"abc def ghi jkl mn "[0, 4, 8, 12, 16, 19], "opq rst uvw xyz"[0, 4, 8, 12, 15] +"abc def ghi jkl mn "[0, 4, 8, 12, 16], "opq rst uvw xyz"[0, 4, 8, 12, 15] Move left by one word "opq rst uvw xyz"[15, 12, 8, 4, 0], "abc def ghi jkl mn "[16, 12, 8, 4, 0] Test 7, RTL: Move left by one word -"abc def ghi jkl mn "[0, 3, 8, 11, 16, 19], "opq rst uvw xyz"[0, 3, 8, 11, 15] +"abc def ghi jkl mn "[0, 3, 8, 11, 16], "opq rst uvw xyz"[0, 3, 8, 11, 15] Move right by one word "opq rst uvw xyz"[15, 11, 8, 3, 0], "abc def ghi jkl mn "[16, 11, 8, 3, 0] Seems that the ICU implementation is not considering the blank space at the end of lines as a word break. Is it expected?
Sorry for replying late. I think gtk's result is more reasonable (in terms of consistent with other output in other tests). Since both gtk and chromium uses ICU, I will need to check why chromium's result is different. (In reply to comment #5) > (In reply to comment #2) > > It is added in r110965 > > under Source/WebCore/platform/text/TextBreakIterator.h. > > > > If you look at its implementation under TextBreakIteratorICU.cpp, > > basically, it is checking the current word break iterator just pass over a word. > > For example, "abc def", both positions "abc| |def" are word break positions, > > but isWordTextBreak returns true for "abc| def" since the break iterator just pass over "abc". And it returns false for "abc |def" since the break iterator just pass over a space. > > > > If GTK+ uses ICU, that should be easy to implemented. > > I've tried running the Skipped tests, and all but one run without problems in webkitgtk, as it uses ICU backend. > > The only test failing is editing/selection/move-by-word-visually-multi-line, with this kind of failure: > > Test 1, LTR: > Move right by one word > -"abc def ghi jkl mn "[0, 4, 8, 12, 16, 19], "opq rst uvw xyz"[0, 4, 8, 12, 15] > +"abc def ghi jkl mn "[0, 4, 8, 12, 16], "opq rst uvw xyz"[0, 4, 8, 12, 15] > Move left by one word > "opq rst uvw xyz"[15, 12, 8, 4, 0], "abc def ghi jkl mn "[16, 12, 8, 4, 0] > > > Test 7, RTL: > Move left by one word > -"abc def ghi jkl mn "[0, 3, 8, 11, 16, 19], "opq rst uvw xyz"[0, 3, 8, 11, 15] > +"abc def ghi jkl mn "[0, 3, 8, 11, 16], "opq rst uvw xyz"[0, 3, 8, 11, 15] > Move right by one word > "opq rst uvw xyz"[15, 11, 8, 3, 0], "abc def ghi jkl mn "[16, 11, 8, 3, 0] > > Seems that the ICU implementation is not considering the blank space at the end of lines as a word break. Is it expected?
Created attachment 165437 [details] Unskipping passing tests
Comment on attachment 165437 [details] Unskipping passing tests I tested it and works, so r=me.
Comment on attachment 165437 [details] Unskipping passing tests Rejecting attachment 165437 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/14038354
Created attachment 165827 [details] Fi
Comment on attachment 165827 [details] Fi fixed changelog
Comment on attachment 165827 [details] Fi Clearing flags on attachment: 165827 Committed r129671: <http://trac.webkit.org/changeset/129671>
Comment on attachment 165827 [details] Fi Ooops, it was already landed.
Yay. Is the bug fixed?
If it is fixed for QT, how about change the bug to WinCE only and keep it open?
Ossy, Tullio is investigating one of the two remaining tests: editing/selection/move-by-word-visually-multi-line.html I think we can keep this bug open for instance. I believe this came after Qt started to use ICU. :-)
Created attachment 166937 [details] Unskipping last tests The problem in this last tests was at the Hebrew fonts. The changes at testfonts [1] made this tests pass now. If this patch pass at bots, i think that this bug is fixed. [1] https://gitorious.org/qtwebkit/testfonts/merge_requests/1
Comment on attachment 166937 [details] Unskipping last tests Clearing flags on attachment: 166937 Committed r130788: <http://trac.webkit.org/changeset/130788>
All reviewed patches have been landed. Closing bug.