Bug 86573 - [chromium] Get text direction at the start and end of selection.
Summary: [chromium] Get text direction at the start and end of selection.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hao Zheng
URL:
Keywords:
Depends on:
Blocks: 66687
  Show dependency tree / graph
 
Reported: 2012-05-15 21:56 PDT by Hao Zheng
Modified: 2012-05-16 22:17 PDT (History)
6 users (show)

See Also:

Attachments
Patch (4.18 KB, patch)
2012-05-15 22:46 PDT, Hao Zheng 		no flags Details | Formatted Diff | Diff
Wrong behavior currently without the change. (317.55 KB, image/png)
2012-05-15 22:48 PDT, Hao Zheng 		no flags Details
Patch (4.15 KB, patch)
2012-05-16 20:10 PDT, Hao Zheng 		no flags Details | Formatted Diff | Diff
Patch (4.10 KB, patch)
2012-05-16 20:51 PDT, Hao Zheng 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Hao Zheng 2012-05-15 21:56:36 PDT 
[chromium] Get text direction at the start and end of selection.
Comment 1 Hao Zheng 2012-05-15 22:46:40 PDT 
Created attachment 142152 [details]
Patch
Comment 2 Hao Zheng 2012-05-15 22:48:23 PDT 
Created attachment 142154 [details]
Wrong behavior currently without the change.

Screenshot to show the problem.
Comment 3 WebKit Review Bot 2012-05-15 22:57:13 PDT 
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 4 Adam Barth 2012-05-16 19:33:22 PDT 
Comment on attachment 142152 [details]
Patch

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

I wonder at what point it makes sense to split off a WebSelection object that has all these APIs.

> Source/WebKit/chromium/public/WebWidget.h:180
> +#if defined(ANDROID)

Please remove this ifdef.  This API isn't Android specific.
Comment 5 Hao Zheng 2012-05-16 20:10:30 PDT 
Created attachment 142399 [details]
Patch
Comment 6 Hao Zheng 2012-05-16 20:12:35 PDT 
(In reply to comment #4)
> (From update of attachment 142152 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142152&action=review
> 
> I wonder at what point it makes sense to split off a WebSelection object that has all these APIs.
> 
> > Source/WebKit/chromium/public/WebWidget.h:180
> > +#if defined(ANDROID)
> 
> Please remove this ifdef.  This API isn't Android specific.

Done. Yes, maybe ChromeOS with touch screen also needs it.
Comment 7 Adam Barth 2012-05-16 20:49:15 PDT 
Comment on attachment 142399 [details]
Patch

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

> Source/WebKit/chromium/src/WebViewImpl.cpp:1976
> +#if OS(ANDROID)

Sorry, I meant we should remove all the ANDROID ifdefs from this patch.
Comment 8 Hao Zheng 2012-05-16 20:51:59 PDT 
Created attachment 142406 [details]
Patch
Comment 9 Hao Zheng 2012-05-16 20:52:42 PDT 
(In reply to comment #7)
> (From update of attachment 142399 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142399&action=review
> 
> > Source/WebKit/chromium/src/WebViewImpl.cpp:1976
> > +#if OS(ANDROID)
> 
> Sorry, I meant we should remove all the ANDROID ifdefs from this patch.

No problem. Thanks for reviewing!
Comment 10 Adam Barth 2012-05-16 20:53:19 PDT 
Comment on attachment 142406 [details]
Patch

Great.  Thank you!
Comment 11 WebKit Review Bot 2012-05-16 22:17:42 PDT 
Comment on attachment 142406 [details]
Patch

Clearing flags on attachment: 142406

Committed r117398: <http://trac.webkit.org/changeset/117398>
Comment 12 WebKit Review Bot 2012-05-16 22:17:47 PDT 
All reviewed patches have been landed.  Closing bug.