RESOLVED FIXED 86573
[chromium] Get text direction at the start and end of selection. 
https://bugs.webkit.org/show_bug.cgi?id=86573
Summary [chromium] Get text direction at the start and end of selection.
Hao Zheng
Reported 2012-05-15 21:56:36 PDT
[chromium] Get text direction at the start and end of selection.
Attachments
Patch (4.18 KB, patch)
2012-05-15 22:46 PDT, Hao Zheng 		no flags
DetailsFormatted DiffDiff
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
DetailsFormatted DiffDiff
Patch (4.10 KB, patch)
2012-05-16 20:51 PDT, Hao Zheng 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Hao Zheng
Comment 1 2012-05-15 22:46:40 PDT
Created attachment 142152 [details] Patch
Hao Zheng
Comment 2 2012-05-15 22:48:23 PDT
Created attachment 142154 [details] Wrong behavior currently without the change. Screenshot to show the problem.
WebKit Review Bot
Comment 3 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.
Adam Barth
Comment 4 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.
Hao Zheng
Comment 5 2012-05-16 20:10:30 PDT
Created attachment 142399 [details] Patch
Hao Zheng
Comment 6 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.
Adam Barth
Comment 7 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.
Hao Zheng
Comment 8 2012-05-16 20:51:59 PDT
Created attachment 142406 [details] Patch
Hao Zheng
Comment 9 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!
Adam Barth
Comment 10 2012-05-16 20:53:19 PDT
Comment on attachment 142406 [details] Patch Great. Thank you!
WebKit Review Bot
Comment 11 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>
WebKit Review Bot
Comment 12 2012-05-16 22:17:47 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.