Bug 101499 - Chromium Android port should consider "." as a word delimiter in English
Summary: Chromium Android port should consider "." as a word delimiter in English
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Xianzhu Wang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-07 11:44 PST by Xianzhu Wang
Modified: 2012-11-13 17:17 PST (History)
9 users (show)

See Also:


Attachments
Patch (3.52 KB, patch)
2012-11-07 12:26 PST, Xianzhu Wang
no flags Details | Formatted Diff | Diff
Patch (11.63 KB, patch)
2012-11-08 10:28 PST, Xianzhu Wang
rniwa: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xianzhu Wang 2012-11-07 11:44:38 PST
On Android there are some special requirements about word selection. When the selection is to contain all white spaces, select an empty range instead to allow pasting. We also want to treat '.' as delimiters of words even when there is no space after the '.'.
Comment 1 Xianzhu Wang 2012-11-07 12:26:34 PST
Created attachment 172858 [details]
Patch
Comment 2 Adam Barth 2012-11-07 13:22:39 PST
It's surprising that we're doing this work in the WebKit layer.  Most other OS-specific selection handling is done in the editing code.
Comment 3 Adam Barth 2012-11-07 13:23:05 PST
Comment on attachment 172858 [details]
Patch

Marking r- for now based on the comment above.
Comment 4 Ryosuke Niwa 2012-11-07 13:25:36 PST
Comment on attachment 172858 [details]
Patch

Yeah this definitely doesn’t belong in WebKit layer.
Comment 5 Xianzhu Wang 2012-11-08 10:28:52 PST
Created attachment 173067 [details]
Patch
Comment 6 Adam Barth 2012-11-08 18:46:53 PST
@rniwa: Would you be willing to review this patch?  I'm unsure if this is the best way to add OS-specific behavior to visible selection.
Comment 7 Ryosuke Niwa 2012-11-08 19:02:17 PST
Comment on attachment 173067 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173067&action=review

> Source/WebCore/editing/android/VisibleSelectionAndroid.cpp:51
> +    // Don't expand beyond '.' for word granularity.
> +    VisiblePosition originalStart = m_start;
> +    validate(granularity);

No, this is what you want to modify. You want to do this at much lower level by overriding ICU's behavior.
Modify findNextWordFromIndex in TextBoundaries.cpp. You probably want to add TextBoundariesAndroid.cpp.
Comment 8 Ryosuke Niwa 2012-11-08 19:04:57 PST
By the way, this won't work in CJK where 。is used as a substitute for period. I bet ICU has some options to tweak this.

+xji for ICU expertise, and +ap, +mitz for word break iteration
Comment 9 Ryosuke Niwa 2012-11-08 19:05:03 PST
By the way, this won't work in CJK where 。is used as a substitute for period. I bet ICU has some options to tweak this.

+xji for ICU expertise, and +ap, +mitz for word break iteration
Comment 10 Xianzhu Wang 2012-11-08 19:14:34 PST
(In reply to comment #7)
> (From update of attachment 173067 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173067&action=review
> 
> > Source/WebCore/editing/android/VisibleSelectionAndroid.cpp:51
> > +    // Don't expand beyond '.' for word granularity.
> > +    VisiblePosition originalStart = m_start;
> > +    validate(granularity);
> 
> No, this is what you want to modify. You want to do this at much lower level by overriding ICU's behavior.
> Modify findNextWordFromIndex in TextBoundaries.cpp. You probably want to add TextBoundariesAndroid.cpp.

Per the requirements, all we want to change is the long press word selection behavior, but not other word breaking behavior.  We can discuss the requirements offline.
Comment 11 Ryosuke Niwa 2012-11-08 19:20:23 PST
(In reply to comment #10)
> Per the requirements, all we want to change is the long press word selection behavior, but not other word breaking behavior.  We can discuss the requirements offline.

That's a very interesting requirement. With all due respect, I strongly recommend you to re-consider this feature and possibly get rid of it altogether. I don't know what's special about "." but changing the behavior of word breaking just for the purpose of long press doesn't sound like something we want to support in WebKit.