Copying text closes the keyboard and the text input gets unfocused, forcing virtual keyboard is get…

Copying text closes the keyboard and the text input gets unfocused, forcing virtual keyboard is getting closed.

During copy operation, we are clearing the selection, which is clearing the cursor
as well. Also if the rootEditable element is NULL, we are not setting the textInputType
for ImeAdapter, which force ImeAdapter to dismiss the keyboard.
Now converted the Range selection to caret selection to keep cursor and caret visible, so
that user can continue editing.

BUG=225090
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278954

[AKVT, 2014-06-12 10:58:46]

PTAL.
Dependent Blink change is at https://codereview.chromium.org/330613002/

[bulach, 2014-06-12 12:41:42]

thanks! +tedchoc is a better reviewer and listed in the original bug

[Ted C, 2014-06-12 18:32:50]

Overall this seems reasonable to me.  Adding aurimas as he's dealt the most with
the editor behavior thus far.

Let's wait for progress on the blink change first though as that is more likely
to be the controversial one.

[AKVT, 2014-06-19 09:22:44]

PTAL this change.

[AKVT, 2014-06-19 09:24:43]

On 2014/06/12 18:32:50, Ted C wrote:
> Overall this seems reasonable to me.  Adding aurimas as he's dealt the most
with
> the editor behavior thus far.
> 
> Let's wait for progress on the blink change first though as that is more
likely
> to be the controversial one.

@tedchoc - Blink team suggested to make changes in Chromium side as the change
is specific to one reason. So I have added the changes in ContentViewCore to
change the selection offset. PTAL

[aurimas (slooooooooow), 2014-06-19 17:12:06]

Can we add a test for this?

https://codereview.chromium.org/330623002/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/330623002/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2093:
mInputConnection.setSelection(selectionEnd, selectionEnd);
Did you test this with every Action (copy, cut, paste, etc)?

[AKVT, 2014-06-20 10:10:48]

@aurimas - Can I add the test inside ContentViewCoreInputConnectionTest class or
I need to add separate class for test ?

https://codereview.chromium.org/330623002/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/330623002/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2093:
mInputConnection.setSelection(selectionEnd, selectionEnd);
On 2014/06/19 17:12:06, aurimas wrote:
> Did you test this with every Action (copy, cut, paste, etc)?

During Copy only unselect() operation will be triggering. So other flow will be
same as it was earlier.
My intention is to block the frame->selection()->clear() which is clearing the
selection and reset the cursor/caret.
With the current change, cursor/caret will be reposition to the end of range
selection.
I have cross verified other scenarios and appears working fine.

[AKVT, 2014-06-20 11:34:56]

PTAL.
@aurimas - Since this is a change which is getting triggered from
SelectActionModeCallback (manual test), Could you please suggest better way of
adding the test.

[AKVT, 2014-06-20 11:53:33]

PTAL

[aurimas (slooooooooow), 2014-06-20 16:23:30]

LGTM given the test passes

[Ted C, 2014-06-20 16:33:57]

lgtm w/ a couple nits

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2095:
else
else should go on the previous line and since the if had braces, the else needs
them as well.

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
File
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
(right):

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:151:
"Sample Text", 11, 11, -1, -1);
+2 indent

[AKVT, 2014-06-20 17:37:27]

The CQ bit was checked by ajith.v@samsung.com

[AKVT, 2014-06-20 17:37:27]

The CQ bit was unchecked by ajith.v@samsung.com

[AKVT, 2014-06-20 18:19:48]

The CQ bit was checked by ajith.v@samsung.com

[commit-bot: I haz the power, 2014-06-20 18:23:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ajith.v@samsung.com/330623002/100001

[Ted C, 2014-06-20 19:03:39]

The CQ bit was unchecked by tedchoc@chromium.org

[Ted C, 2014-06-20 19:04:33]

Please fix the comments before checking the commit queue checkbox

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
File
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
(right):

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:151:
"Sample Text", 11, 11, -1, -1);
On 2014/06/20 16:33:57, Ted C wrote:
> +2 indent

Actually, +4 indent

[AKVT, 2014-06-21 07:20:13]

PTAL

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2095:
else
On 2014/06/20 16:33:57, Ted C wrote:
> else should go on the previous line and since the if had braces, the else
needs
> them as well.

Done.

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
File
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
(right):

https://codereview.chromium.org/330623002/diff/100001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:151:
"Sample Text", 11, 11, -1, -1);
On 2014/06/20 19:04:33, Ted C wrote:
> On 2014/06/20 16:33:57, Ted C wrote:
> > +2 indent
> 
> Actually, +4 indent

Done.

[AKVT, 2014-06-21 07:20:46]

The CQ bit was checked by ajith.v@samsung.com

[commit-bot: I haz the power, 2014-06-21 07:21:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ajith.v@samsung.com/330623002/120001

[commit-bot: I haz the power, 2014-06-21 13:14:46]

Change committed as 278954