Support parsing BackgroundSpans and UnderlineSpans in Android IME'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 4debf150004b75497f83efa480177e71d5c8a50a..78dc4862c92f4eb431be9f0c6fde1cd08f52f5aa 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -267,21 +267,28 @@ bool InputMethodController::finishComposingText(
     return result;
   }
 
-  return replaceCompositionAndMoveCaret(composingText(), 0);
+  return replaceCompositionAndMoveCaret(composingText(), 0,
+                                        Vector<CompositionUnderline>());
 }
 
-bool InputMethodController::commitText(const String& text,
-                                       int relativeCaretPosition) {
-  if (hasComposition())
-    return replaceCompositionAndMoveCaret(text, relativeCaretPosition);
+bool InputMethodController::commitText(
+    const String& text,
+    const Vector<CompositionUnderline>& underlines,
+    int relativeCaretPosition) {
+  if (hasComposition()) {
+    return replaceCompositionAndMoveCaret(text, relativeCaretPosition,
+                                          underlines);
+  }
 
   // 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)
+  if (!text.length() && !relativeCaretPosition) {
     return false;
-  return insertTextAndMoveCaret(text, relativeCaretPosition);
+  }
+
+  return insertTextAndMoveCaret(text, relativeCaretPosition, underlines);
 }
 
 bool InputMethodController::replaceComposition(const String& text) {
@@ -324,9 +331,27 @@ static int computeAbsoluteCaretPosition(size_t textStart,
   return textStart + textLength + relativeCaretPosition;
 }
 
+void InputMethodController::addCompositionUnderlines(
+    const Vector<CompositionUnderline>& underlines,
+    Node* baseNode,
+    unsigned baseOffset) {
+  for (const auto& underline : underlines) {
+    unsigned underlineStart = baseOffset + underline.startOffset();
+    unsigned underlineEnd = baseOffset + underline.endOffset();
+    EphemeralRange ephemeralLineRange = EphemeralRange(
+        Position(baseNode, underlineStart), Position(baseNode, underlineEnd));
+    if (ephemeralLineRange.isNull())
+      continue;
+    document().markers().addCompositionMarker(
+        ephemeralLineRange.startPosition(), ephemeralLineRange.endPosition(),
+        underline.color(), underline.thick(), underline.backgroundColor());
+  }
+}
+
 bool InputMethodController::replaceCompositionAndMoveCaret(
     const String& text,
-    int relativeCaretPosition) {
+    int relativeCaretPosition,
+    const Vector<CompositionUnderline>& underlines) {
   Element* rootEditableElement = frame().selection().rootEditableElement();
   if (!rootEditableElement)
     return false;
@@ -340,6 +365,8 @@ bool InputMethodController::replaceCompositionAndMoveCaret(
   if (!replaceComposition(text))
     return false;
 
+  addCompositionUnderlines(underlines, rootEditableElement, textStart);

[rlanday, 2016/12/16 20:55:33]

I don't think the Voice IME actually ever replaces a composition span with text
containing underlines, I think it always blanks out the existing text first. So
I don't know that this actually works properly, but I assume we should support
this?

This code uses rootEditableElement() as opposed to the base node of the
selection. rootEditableElement() didn't seem to work properly in
insertTextAndMoveCaret() and I don't really understand why.

[rlanday, 2016/12/16 21:01:35]

I'm going to try to add a test case to InputMethodControllerTest.cpp that tests
this behavior, that should solve this problem...

[rlanday, 2016/12/16 21:01:35]

I'm going to try to add a test case to InputMethodControllerTest.cpp that tests
this behavior, that should solve this problem...

+
   int absoluteCaretPosition = computeAbsoluteCaretPosition(
       textStart, text.length(), relativeCaretPosition);
   if (!moveCaret(absoluteCaretPosition))
@@ -359,16 +386,28 @@ bool InputMethodController::insertText(const String& text) {
   return true;
 }
 
-bool InputMethodController::insertTextAndMoveCaret(const String& text,
-                                                   int relativeCaretPosition) {
+bool InputMethodController::insertTextAndMoveCaret(
+    const String& text,
+    int relativeCaretPosition,
+    const Vector<CompositionUnderline>& underlines) {
   PlainTextRange selectionRange = getSelectionOffsets();
   if (selectionRange.isNull())
     return false;
   int textStart = selectionRange.start();
 
   if (text.length()) {
+    Position base = mostForwardCaretPosition(frame().selection().base());
+    Node* baseNode = base.anchorNode();
+
+    if (!baseNode)

[rlanday, 2016/12/16 20:55:33]

Note: if I add the baseNode->isTextNode() check that insertText() has, this
fails on a node whose path looks like this:
/HTML[0]/BODY[0]/FORM[@id="textform" and position()=0]/TEXTAREA[@id="text" and
position()=0]/#shadow-root[0]/DIV[@id="inner-editor"]

Not sure exactly what's going on.

+      return false;
+
+    unsigned baseOffset = base.computeOffsetInContainerNode();
+
     if (!insertText(text))
       return false;
+
+    addCompositionUnderlines(underlines, baseNode, baseOffset);
   }
 
   int absoluteCaretPosition = computeAbsoluteCaretPosition(
@@ -578,17 +617,8 @@ void InputMethodController::setComposition(
         LayoutTheme::theme().platformDefaultCompositionBackgroundColor());
     return;
   }
-  for (const auto& underline : underlines) {
-    unsigned underlineStart = baseOffset + underline.startOffset();
-    unsigned underlineEnd = baseOffset + underline.endOffset();
-    EphemeralRange ephemeralLineRange = EphemeralRange(
-        Position(baseNode, underlineStart), Position(baseNode, underlineEnd));
-    if (ephemeralLineRange.isNull())
-      continue;
-    document().markers().addCompositionMarker(
-        ephemeralLineRange.startPosition(), ephemeralLineRange.endPosition(),
-        underline.color(), underline.thick(), underline.backgroundColor());
-  }
+
+  addCompositionUnderlines(underlines, baseNode, baseOffset);
 }
 
 PlainTextRange InputMethodController::createSelectionRangeForSetComposition(
@@ -627,17 +657,7 @@ void InputMethodController::setCompositionFromExistingText(
 
   clear();
 
-  for (const auto& underline : underlines) {
-    unsigned underlineStart = compositionStart + underline.startOffset();
-    unsigned underlineEnd = compositionStart + underline.endOffset();
-    EphemeralRange ephemeralLineRange =
-        PlainTextRange(underlineStart, underlineEnd).createRange(*editable);
-    if (ephemeralLineRange.isNull())
-      continue;
-    document().markers().addCompositionMarker(
-        ephemeralLineRange.startPosition(), ephemeralLineRange.endPosition(),
-        underline.color(), underline.thick(), underline.backgroundColor());
-  }
+  addCompositionUnderlines(underlines, editable, compositionStart);
 
   m_hasComposition = true;
   if (!m_compositionRange)