Handle newCursorPosition correctly for Android's commitText()

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

 /*
  * Copyright (C) 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 165 matching lines @@
     if (range.isNull())
         return;
 
     // The composition can start inside a composed character sequence, so we have to override checks.
     // See <http://bugs.webkit.org/show_bug.cgi?id=15781>
     VisibleSelection selection;
     selection.setWithoutValidation(range.startPosition(), range.endPosition());
     frame().selection().setSelection(selection, 0);
 }
 
-bool InputMethodController::confirmComposition()
-{
-    return confirmComposition(composingText());
-}
-
-bool InputMethodController::confirmComposition(const String& text, ConfirmCompositionBehavior confirmBehavior)
+bool InputMethodController::finishComposingText(ConfirmCompositionBehavior confirmBehavior)
 {
     if (!hasComposition())
         return false;
+
+    if (confirmBehavior == KeepSelection) {
+        SelectionOffsetsScope selectionOffsetsScope(this);
+        return replaceComposition(composingText(), KeepSelection);
+    }
+
+    return replaceCompositionAndMoveCaret(composingText(), 0);
+}
+
+bool InputMethodController::commitText(const String& text, int relativeCaretPosition)
+{
+    if (!hasComposition()) {
+        // We should do nothing in this case, because:
+        // 1. No need to insert text when text is empty.
+        // 2. Shouldn't move caret when relativeCaretPosition == 0 to avoid
+        // duplicate selection change event.
+        if (!text.length() && !relativeCaretPosition)
+            return false;
+        return insertTextAndMoveCaret(text, relativeCaretPosition);
+    }
+
+    return replaceCompositionAndMoveCaret(text, relativeCaretPosition);

[yosin_UTC9, 2016/09/07 07:19:20]

nit: It is better to switch if's condition to reduce indentation:

if (hasComposiiton())
    return replaceCompositionAndMoveCaret(text, relativeCaretPosition);

// We should do nothing in this case, because:
// 1. No need to insert text when text is empty.
// 2. Shouldn't move caret when relativeCaretPosition == 0 to avoid
// duplicate selection change event.
if (!text.length() && !relativeCaretPosition)
    return false;
return insertTextAndMoveCaret(text, relativeCaretPosition);

[yabinh, 2016/09/07 10:27:15]

Done.

+}
+
+bool InputMethodController::replaceComposition(const String& text, ConfirmCompositionBehavior confirmBehavior)
+{
+    if (!hasComposition())
+        return false;
 
     Optional<Editor::RevealSelectionScope> revealSelectionScope;
     if (confirmBehavior == KeepSelection)
         revealSelectionScope.emplace(&editor());

[aelias_OOO_until_Jul13, 2016/09/07 05:02:40]

Please move this to finishComposingText() and remove the argument from
replaceComposition().

[yabinh, 2016/09/07 10:27:15]

Done.
Note that setComposition() will also be affected.

 
     // If the composition was set from existing text and didn't change, then
     // there's nothing to do here (and we should avoid doing anything as that
     // may clobber multi-node styled text).
     if (!m_isDirty && composingText() == text) {
         clear();
         return true;
     }
 
     // Select the text that will be deleted or replaced.
@@ skipping 17 matching lines @@
     // Event handler might destroy document.
     if (!frame().document())
         return false;
 
     // No DOM update after 'compositionend'.
     dispatchCompositionEndEvent(frame(), text);
 
     return true;
 }
 
-bool InputMethodController::confirmCompositionOrInsertText(const String& text, ConfirmCompositionBehavior confirmBehavior)
+// relativeCaretPosition is relative to the end of the text.
+static int computeAbsoluteCaretPosition(size_t textStart, size_t textLength, int relativeCaretPosition)
 {
-    if (!hasComposition()) {
-        if (!text.length())
+    return textStart + textLength + relativeCaretPosition;
+}
+
+bool InputMethodController::replaceCompositionAndMoveCaret(const String& text, int relativeCaretPosition)
+{
+    Element* rootEditableElement = frame().selection().rootEditableElement();
+    if (!rootEditableElement)
+        return false;
+    PlainTextRange compositionRange = PlainTextRange::create(*rootEditableElement, *m_compositionRange);
+    if (compositionRange.isNull())
+        return false;
+    int textStart = compositionRange.start();
+
+    if (!replaceComposition(text, DoNotKeepSelection))
+        return false;
+
+    int absoluteCaretPosition = computeAbsoluteCaretPosition(textStart, text.length(), relativeCaretPosition);
+    return moveCaret(absoluteCaretPosition);
+}
+
+bool InputMethodController::insertText(const String& text)
+{
+    if (dispatchBeforeInputInsertText(frame().document()->focusedElement(), text) != DispatchEventResult::NotCanceled)
+        return false;
+    editor().insertText(text, 0);
+    return true;
+}
+
+bool InputMethodController::insertTextAndMoveCaret(const String& text, int relativeCaretPosition)
+{
+    PlainTextRange selectionRange = getSelectionOffsets();
+    if (selectionRange.isNull())
+        return false;
+    int textStart = selectionRange.start();
+
+    if (text.length()) {
+        if (!insertText(text))
             return false;
-
-        if (dispatchBeforeInputInsertText(frame().document()->focusedElement(), text) != DispatchEventResult::NotCanceled)
-            return false;
-
-        editor().insertText(text, 0);
-        return true;
     }
 
-    if (text.length()) {
-        confirmComposition(text);
-        return true;
-    }
-
-    if (confirmBehavior == DoNotKeepSelection)
-        return confirmComposition(composingText(), DoNotKeepSelection);
-
-    SelectionOffsetsScope selectionOffsetsScope(this);
-    return confirmComposition();
+    int absoluteCaretPosition = computeAbsoluteCaretPosition(textStart, text.length(), relativeCaretPosition);
+    return moveCaret(absoluteCaretPosition);
 }
 
 void InputMethodController::cancelComposition()
 {
     if (!hasComposition())
         return;
 
     Editor::RevealSelectionScope revealSelectionScope(&editor());
 
     if (frame().selection().isNone())
@@ skipping 67 matching lines @@
     //    Sending a compositionupdate event at this time ensures that at least one
     //    compositionupdate event is dispatched.
     // 2. Updating the existing composition node.
     //    Send a compositionupdate event when this function updates the existing composition
     //    node, i.e. hasComposition() && !text.isEmpty().
     // 3. Canceling the ongoing composition.
     //    Send a compositionend event when function deletes the existing composition node, i.e.
     //    !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.
             TypingCommand::deleteSelection(*frame().document(), TypingCommand::PreventSpellChecking);
         }
 
         setEditableSelectionOffsets(selectedRange);
         return;
     }
 
@@ skipping 180 matching lines @@
 
     rightBoundary += textLength;
 
     // In case of exceeding the right boundary.
     start = std::min(start, rightBoundary);
     end = std::min(end, rightBoundary);
 
     return PlainTextRange(start, end);
 }
 
+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())
         return;
     PlainTextRange selectionOffsets(getSelectionOffsets());
     if (selectionOffsets.isNull())
         return;
 
     // A common call of before=1 and after=0 will fail if the last character
     // is multi-code-word UTF-16, including both multi-16bit code-points and
@@ skipping 19 matching lines @@
     TypingCommand::deleteSelection(*frame().document());
 }
 
 DEFINE_TRACE(InputMethodController)
 {
     visitor->trace(m_frame);
     visitor->trace(m_compositionRange);
 }
 
 } // namespace blink