Handle newCursorPosition correctly for Android's commitText()

third_party/WebKit/Source/core/editing/InputMethodController.cpp

Index: third_party/WebKit/Source/core/editing/InputMethodController.cpp
diff --git a/third_party/WebKit/Source/core/editing/InputMethodController.cpp b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
index 343e89aad7a0aaf3644dca5f79449d6d75138bc9..6f514818bd6b9846e5075e177b440aeee66425e8 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -185,10 +185,10 @@ void InputMethodController::selectComposition() const
 
 bool InputMethodController::confirmComposition()
 {
-    return confirmComposition(composingText());
+    return replaceComposition(composingText(), KeepSelection);
 }
 
-bool InputMethodController::confirmComposition(const String& text, ConfirmCompositionBehavior confirmBehavior)
+bool InputMethodController::replaceComposition(const String& text, ConfirmCompositionBehavior confirmBehavior)
 {
     if (!hasComposition())
         return false;
@@ -233,31 +233,66 @@ bool InputMethodController::confirmComposition(const String& text, ConfirmCompos
     return true;
 }
 
-bool InputMethodController::confirmCompositionOrInsertText(const String& text, ConfirmCompositionBehavior confirmBehavior)
+bool InputMethodController::replaceCompositionAndMoveCaret(const String& text, int relativeCaretPosition)
 {
-    if (!hasComposition()) {
-        if (!text.length())
-            return false;
+    Element* rootEditableElement = frame().selection().rootEditableElement();
+    if (!rootEditableElement)
+        return false;
+    PlainTextRange compositionRange = PlainTextRange::create(*rootEditableElement, *m_compositionRange);
+    if (compositionRange.isNull())
+        return false;
 
-        if (dispatchBeforeInputInsertText(frame().document()->focusedElement(), text) != DispatchEventResult::NotCanceled)
-            return false;
+    int absoluteCaretPosition = getAbsoluteCaretPosition(compositionRange.start(), text.length(), relativeCaretPosition);
+    return replaceComposition(text, DoNotKeepSelection) && moveCaret(absoluteCaretPosition);
+}
 
-        editor().insertText(text, 0);
-        return true;
-    }
+bool InputMethodController::commitComposition(const String& text)
+{
+    if (!hasComposition())
+        return insertText(text);
 
     if (text.length()) {
-        confirmComposition(text);
+        replaceComposition(text, KeepSelection);
         return true;
     }
 
-    if (confirmBehavior == DoNotKeepSelection)
-        return confirmComposition(composingText(), DoNotKeepSelection);
-
     SelectionOffsetsScope selectionOffsetsScope(this);
     return confirmComposition();
 }
 
+bool InputMethodController::commitCompositionAndMoveCaret(const String& text, int relativeCaretPosition)
+{
+    if (!hasComposition())
+        return insertTextAndMoveCaret(text, relativeCaretPosition);
+

[yosin_UTC9, 2016/08/29 07:02:01]

nit: no need to have an extra blank line.

[yabinh, 2016/08/29 08:44:00]

Done.

+    if (text.length())
+        return replaceCompositionAndMoveCaret(text, relativeCaretPosition);
+

[yosin_UTC9, 2016/08/29 07:02:02]

nit: no need to have an extra blank line.

[yabinh, 2016/08/29 08:44:01]

Done.

+    return replaceCompositionAndMoveCaret(composingText(), relativeCaretPosition);
+}
+
+bool InputMethodController::insertText(const String& text)
+{
+    if (!text.length())
+        return false;
+

[yosin_UTC9, 2016/08/29 07:02:02]

nit: no need to have an extra blank line.

[yabinh, 2016/08/29 08:44:01]

Done.

+    if (dispatchBeforeInputInsertText(frame().document()->focusedElement(), text) != DispatchEventResult::NotCanceled)
+        return false;
+

[yosin_UTC9, 2016/08/29 07:02:02]

nit: no need to have an extra blank line.

[yabinh, 2016/08/29 08:44:01]

Done.

+    editor().insertText(text, 0);
+    return true;
+}
+
+bool InputMethodController::insertTextAndMoveCaret(const String& text, int relativeCaretPosition)
+{
+    PlainTextRange selectionRange = getSelectionOffsets();
+    if (selectionRange.isNull())
+        return false;
+

[yosin_UTC9, 2016/08/29 07:02:02]

nit: no need to have an extra blank line.

[yabinh, 2016/08/29 08:44:01]

Done.

+    int absoluteCaretPosition = getAbsoluteCaretPosition(selectionRange.start(), text.length(), relativeCaretPosition);

[yosin_UTC9, 2016/08/29 07:02:01]

s/getAbsoluteCaretPosition/computeAbsoluteCaretPosition/
Blink doesn't prefer function names starting with "get".

[yabinh, 2016/08/29 08:44:01]

Done.

+    return insertText(text) && moveCaret(absoluteCaretPosition);

[yosin_UTC9, 2016/08/29 07:02:02]

We should not compute |absoluteCaretPosition| when |insertText(text)| returns
false.

[yabinh, 2016/08/29 08:44:01]

Done.
BTW, we should restore the selection and composition data before inserting text,
because they will change after insertion.

+}
+
 void InputMethodController::cancelComposition()
 {
     if (!hasComposition())
@@ -343,7 +378,7 @@ void InputMethodController::setComposition(const String& text, const Vector<Comp
     //    !hasComposition() && test.isEmpty().
     if (text.isEmpty()) {
         if (hasComposition()) {
-            confirmComposition(emptyString());
+            replaceComposition(emptyString(), KeepSelection);
         } else {
             // It's weird to call |setComposition()| with empty text outside composition, however some IME
             // (e.g. Japanese IBus-Anthy) did this, so we simply delete selection without sending extra events.
@@ -543,6 +578,27 @@ PlainTextRange InputMethodController::createRangeForSelection(int start, int end
     return PlainTextRange(start, end);
 }
 
+int InputMethodController::getAbsoluteCaretPosition(size_t textStart, size_t textLength, int relativeCaretPosition) const

[yosin_UTC9, 2016/08/29 07:02:02]

This function should be a local static function instead of a member function of
|InputMethodController|, since this function doesn't use members in
|InputMethodController| class.

[yabinh, 2016/08/29 08:44:00]

Done.

+{
+    // If relativeCaretPosition > 0, it's relative to the end of the text - 1;

[yosin_UTC9, 2016/08/29 07:02:01]

Please put this comment in .h file.

[yabinh, 2016/08/29 08:44:01]

Since this function is not declared in .h file in the latest patch, I'll keep
the comments in .cpp file.

+    // if relativeCaretPosition <= 0, it is relative to the start of the text.
+    // This is to match Android behavior. See
+    // https://developer.android.com/reference/android/view/inputmethod/InputConnection.html#commitText(java.lang.CharSequence, int)
+    if (relativeCaretPosition > 0)
+        relativeCaretPosition += textLength - 1;
+    return textStart + relativeCaretPosition;
+}
+
+bool InputMethodController::moveCaret(int newCaretPosition)
+{
+    frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
+
+    PlainTextRange selectedRange = createRangeForSelection(newCaretPosition, newCaretPosition, 0);
+    if (selectedRange.isNull())
+        return false;
+    return setEditableSelectionOffsets(selectedRange);
+}
+
 void InputMethodController::extendSelectionAndDelete(int before, int after)
 {
     if (!editor().canEdit())