[WIP] Add support for Android SuggestionSpans when editing text

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 15 matching lines @@
 
 #include "core/editing/InputMethodController.h"
 
 #include "core/InputModeNames.h"
 #include "core/InputTypeNames.h"
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/dom/Text.h"
 #include "core/editing/EditingUtilities.h"
 #include "core/editing/Editor.h"
+#include "core/editing/TextSuggestionList.h"
 #include "core/editing/commands/TypingCommand.h"
 #include "core/editing/markers/DocumentMarkerController.h"
 #include "core/events/CompositionEvent.h"
 #include "core/frame/LocalFrame.h"
 #include "core/html/HTMLInputElement.h"
 #include "core/html/HTMLTextAreaElement.h"
 #include "core/input/EventHandler.h"
 #include "core/layout/LayoutObject.h"
 #include "core/layout/LayoutTheme.h"
 #include "core/page/ChromeClient.h"
 
 namespace blink {
 
 namespace {
 
+const int kMaxNumberSuggestions = 5;

[esprehn, 2017/01/31 22:41:34]

Why 5?

[rlanday, 2017/01/31 23:30:09]

5 is what the native text box on Android uses. Blink has to be aware of this
number because once we find 5 suggestions, we stop looking for longer and longer
ranges that the cursor is over to pull more suggestions. E.g. if we have:

The quick brown fox jumped over the lazy dog.

and we have the following suggestion ranges:

"The": 1 suggestion
"The quick": 1 suggestion
"The quick brown": 1 suggestion
"The quick brown fox": 1 suggestion
"The quick brown fox jumped:" 1 suggestion
"The quick brown fox jumped over:" 1 suggestion

and you tap on "The", we highlight up to "The quick brown fox jumped" since
that's how far we have to go to get 5 suggestions.

+const double kSuggestionUnderlineAlphaMultiplier = 0.4;

[rlanday, 2017/01/31 19:50:21]

The suggestion highlight background seems kind of hard to see on the yellow
background of the webpage I've been using for testing (editpad.org), but this is
the multiplier the native Android text box uses...

[esprehn, 2017/01/31 22:41:34]

Did this get a UI review?

[rlanday, 2017/01/31 23:30:09]

This is just existing Android UI we're porting over, so it didn't seem
necessary. Do you disagree?

+
 void dispatchCompositionUpdateEvent(LocalFrame& frame, const String& text) {
   Element* target = frame.document()->focusedElement();
   if (!target)
     return;
 
   CompositionEvent* event = CompositionEvent::create(
       EventTypeNames::compositionupdate, frame.domWindow(), text);
   target->dispatchEvent(event);
 }
 
 void dispatchCompositionEndEvent(LocalFrame& frame, const String& text) {
   Element* target = frame.document()->focusedElement();
   if (!target)
     return;
 
   CompositionEvent* event = CompositionEvent::create(
       EventTypeNames::compositionend, frame.domWindow(), text);
   target->dispatchEvent(event);
 }
 
 bool needsIncrementalInsertion(const LocalFrame& frame, const String& newText) {
-  // No need to apply incremental insertion if it doesn't support formated text.
-  if (!frame.editor().canEditRichly())
-    return false;
-
   // No need to apply incremental insertion if the old text (text to be
   // replaced) or the new text (text to be inserted) is empty.
   if (frame.selectedText().isEmpty() || newText.isEmpty())
     return false;
 
   return true;
 }
 
 DispatchEventResult dispatchBeforeInputFromComposition(
     EventTarget* target,
@@ skipping 212 matching lines @@
   // 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, underlines);
 }
 
+void InputMethodController::applySuggestionReplacement(int documentMarkerID,

[esprehn, 2017/01/31 22:41:34]

This change is so huge, we probably need a lot of tests. Why is this different
than how normal spell check suggestions are applied?

[rlanday, 2017/01/31 23:30:09]

Yeah I should write some tests for this method.

Basically it's different from spell check in that, for spell check, the markers
do not overlap, and can be regenerated whenever we want by re-running the spell
check. For this, the markers are added by the IME, can overlap and/or be nested,
and cannot be regenerated by blink (the IME can rewrite the text but we can't
control that), so we have to carefully persist them, and keep them updated
properly when performing replacements.

+                                                       int suggestionIndex) {
+  suggestionMenuClosed();
+
+  Element* rootEditableElement = frame().selection().rootEditableElement();
+  DocumentMarkerVector suggestionMarkers =
+      rootEditableElement->document().markers().markersInRange(

[rlanday, 2017/01/31 21:17:12]

I introduced a bug here when I was refactoring, this should be
markersInRangeInclusive() (to include markers the cursor is immediately before
or immediately after)

+          firstEphemeralRangeOf(m_frame->selection().selection()),
+          DocumentMarker::Suggestion);
+
+  DocumentMarker* markerPtr = nullptr;
+  for (const Member<DocumentMarker>& marker : suggestionMarkers) {
+    if (marker->suggestionMarkerID() == documentMarkerID) {
+      markerPtr = marker.get();
+      break;
+    }
+  }
+
+  if (!markerPtr)
+    return;
+
+  // Remove markers that contain part, but not all, of the text range to be
+  // replaced.
+  Vector<int> markerIdsToRemove;
+  HeapVector<Member<DocumentMarker>> markersToExpand;
+  for (const Member<DocumentMarker>& marker : suggestionMarkers) {
+    if ((marker->startOffset() > markerPtr->startOffset() &&
+         marker->startOffset() <= markerPtr->endOffset()) ||
+        (marker->endOffset() < markerPtr->endOffset() &&
+         marker->endOffset() >= markerPtr->startOffset())) {
+      markerIdsToRemove.push_back(marker->suggestionMarkerID());
+    }
+
+    if (marker->startOffset() == markerPtr->startOffset() &&
+        marker->endOffset() == markerPtr->endOffset()) {
+      // Special case: DocumentMarkerController handles shifting most of the

[aelias_OOO_until_Jul13, 2017/02/10 21:43:55]

As a reminder of what we discussed offline, marking marker ranges "inclusive" is
the way to solve this problem, and this special case should be deleted.

+      // markers around, but if we replace exactly the range of text spanned by
+      // a marker, in some cases, the range will get collapsed to a single
+      // position when the old text is removed and will not be re-expanded. So
+      // we have to re-expand those markers here after the replacement.
+
+      // We actually have to remove and re-insert these markers because in the
+      // case where all the text is replaced, it's possible for the text node
+      // to get removed and re-added, thereby dropping the markers.
+      markerIdsToRemove.push_back(marker->suggestionMarkerID());
+      markersToExpand.push_back(marker);
+    }
+  }
+
+  document().markers().removeSuggestionMarkersByID(markerIdsToRemove);
+
+  setCompositionFromExistingText(Vector<CompositionUnderline>(),

[aelias_OOO_until_Jul13, 2017/02/10 21:43:55]

There's no reason to do anything relating to composition here.  Please select
the word directly without going through composition.  Then it will be easier to
separate this method out from InputMethodController.

+                                 markerPtr->startOffset(),
+                                 markerPtr->endOffset());
+
+  // The entry in the suggestion drop-down that the user tapped on is replaced
+  // by the text they just overwrote
+  String replacement =
+      String::fromUTF8(markerPtr->suggestions()[suggestionIndex].c_str());
+  int replacementLength = replacement.length();
+  String newSuggestion = composingText();
+
+  // Don't use replaceComposition() here because we don't want to send
+  // JavaScript composition events
+  selectComposition();
+  frame().editor().replaceSelectionWithText(
+      replacement, false, false, InputEvent::InputType::InsertReplacementText);
+
+  markerPtr->replaceSuggestion(suggestionIndex, newSuggestion.utf8().data());
+
+  // Could have changed when we replaced the composition, if we deleted all the
+  // text
+  rootEditableElement = frame().selection().rootEditableElement();
+  for (const Member<DocumentMarker>& marker : markersToExpand) {
+    const EphemeralRange& range =
+        PlainTextRange(marker->startOffset(),
+                       marker->startOffset() + replacementLength)
+            .createRange(*rootEditableElement);
+    document().markers().addCompositionMarker(
+        range.startPosition(), range.endPosition(), marker->underlineColor(),
+        marker->thick(), marker->backgroundColor(), marker->suggestions());
+  }
+}
+
+void InputMethodController::deleteSuggestionHighlight() {
+  Element* rootEditableElement = frame().selection().rootEditableElement();
+  DocumentMarkerVector suggestionHighlightMarkers =
+      rootEditableElement->document().markers().markersInRange(
+          firstEphemeralRangeOf(m_frame->selection().selection()),
+          DocumentMarker::SuggestionBackgroundHighlight);
+
+  if (suggestionHighlightMarkers.isEmpty())
+    return;
+
+  const DocumentMarker* suggestionHighlightMarkerPtr =
+      suggestionHighlightMarkers[0].get();
+
+  // Remove markers that contain part, but not all, of the text range to be

[aelias_OOO_until_Jul13, 2017/02/10 21:43:55]

It can't be correct for this code to live here, since there's no difference
between this situation, and the one where you long-press-select the word and
press backspace.  It should be in a more generic place that covers both cases.

+  // deleted.
+  Vector<int> markerIdsToRemove;
+  for (const Member<DocumentMarker>& marker : suggestionHighlightMarkers) {
+    if ((marker->startOffset() > suggestionHighlightMarkerPtr->startOffset() &&
+         marker->startOffset() <= suggestionHighlightMarkerPtr->endOffset()) ||
+        (marker->endOffset() < suggestionHighlightMarkerPtr->endOffset() &&
+         marker->endOffset() >= suggestionHighlightMarkerPtr->startOffset())) {
+      markerIdsToRemove.push_back(marker->suggestionMarkerID());
+    }
+  }
+
+  document().markers().removeSuggestionMarkersByID(markerIdsToRemove);
+
+  // If the character immediately following the range to be deleted is a space,

[aelias_OOO_until_Jul13, 2017/02/10 21:43:55]

Please split this off into a separate method called
"expandRangeToSurroundingWhitespace"

+  // delete it if either of these conditions holds:
+  // - We're deleting at the beginning of the editable text (to avoid ending up
+  //   with a space at the beginning)
+  // - The character immediately before the range being deleted is also a space
+  //   (to avoid ending up with two adjacent spaces)
+
+  bool deleteNextChar = false;
+
+  const PlainTextRange nextCharacterRange(
+      suggestionHighlightMarkerPtr->endOffset(),
+      suggestionHighlightMarkerPtr->endOffset() + 1);
+  const EphemeralRange& nextCharacterEphemeralRange =
+      nextCharacterRange.createRange(*rootEditableElement);
+  if (!nextCharacterEphemeralRange.isNull()) {
+    String nextCharacterStr = plainText(
+        nextCharacterEphemeralRange,
+        TextIteratorBehavior::Builder().setEmitsSpaceForNbsp(true).build());
+
+    if (WTF::isASCIISpace(nextCharacterStr[0])) {
+      if (suggestionHighlightMarkerPtr->startOffset() == 0) {
+        deleteNextChar = true;
+      } else {
+        const PlainTextRange prevCharacterRange(
+            suggestionHighlightMarkerPtr->startOffset() - 1,
+            suggestionHighlightMarkerPtr->startOffset());
+        const EphemeralRange prevCharacterEphemeralRange =
+            prevCharacterRange.createRange(*rootEditableElement);
+        if (!prevCharacterEphemeralRange.isNull()) {
+          String prevCharacterStr = plainText(prevCharacterEphemeralRange,
+                                              TextIteratorBehavior::Builder()
+                                                  .setEmitsSpaceForNbsp(true)
+                                                  .build());
+          if (WTF::isASCIISpace(prevCharacterStr[0])) {
+            deleteNextChar = true;
+          }
+        }
+      }
+    }
+  }
+
+  const EphemeralRange highlightRange =
+      PlainTextRange(suggestionHighlightMarkerPtr->startOffset(),
+                     suggestionHighlightMarkerPtr->endOffset() + deleteNextChar)
+          .createRange(*rootEditableElement);
+
+  frame().selection().setSelection(
+      SelectionInDOMTree::Builder().setBaseAndExtent(highlightRange).build(),
+      0);
+  // Don't want to send JavaScript composition events here
+  frame().editor().replaceSelectionWithText(
+      "", false, false, InputEvent::InputType::InsertReplacementText);
+
+  suggestionMenuClosed();
+}
+
+void InputMethodController::suggestionMenuClosed() {
+  document().markers().removeMarkers(
+      DocumentMarker::SuggestionBackgroundHighlight);
+  frame().selection().setCaretVisible(true);
+}
+
 bool InputMethodController::replaceComposition(const String& text) {
   if (!hasComposition())
     return false;
 
   // Select the text that will be deleted or replaced.
   selectComposition();
 
   if (frame().selection().isNone())
     return false;
 
@@ skipping 37 matching lines @@
     unsigned underlineEnd = offset + underline.endOffset();
 
     EphemeralRange ephemeralLineRange =
         PlainTextRange(underlineStart, underlineEnd)
             .createRange(*rootEditableElement);
     if (ephemeralLineRange.isNull())
       continue;
 
     document().markers().addCompositionMarker(
         ephemeralLineRange.startPosition(), ephemeralLineRange.endPosition(),
-        underline.color(), underline.thick(), underline.backgroundColor());
+        underline.color(), underline.thick(), underline.backgroundColor(),
+        underline.suggestions());
   }
 }
 
+void InputMethodController::clearSuggestionMarkersTouchingSelection() {
+  document().markers().removeMarkersForWordsAffectedByEditing(
+      DocumentMarker::Suggestion, false);
+}
+
 bool InputMethodController::replaceCompositionAndMoveCaret(
     const String& text,
     int relativeCaretPosition,
     const Vector<CompositionUnderline>& underlines) {
   Element* rootEditableElement = frame().selection().rootEditableElement();
   if (!rootEditableElement)
     return false;
   DCHECK(hasComposition());
   PlainTextRange compositionRange =
       PlainTextRange::create(*rootEditableElement, *m_compositionRange);
   if (compositionRange.isNull())
     return false;
+
   int textStart = compositionRange.start();
 
   if (!replaceComposition(text))
     return false;
 
   // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
   // needs to be audited. see http://crbug.com/590369 for more details.
   document().updateStyleAndLayoutIgnorePendingStylesheets();
 
   addCompositionUnderlines(underlines, rootEditableElement, textStart);
@@ skipping 11 matching lines @@
   return true;
 }
 
 bool InputMethodController::insertTextAndMoveCaret(
     const String& text,
     int relativeCaretPosition,
     const Vector<CompositionUnderline>& underlines) {
   PlainTextRange selectionRange = getSelectionOffsets();
   if (selectionRange.isNull())
     return false;
+
+  clearSuggestionMarkersTouchingSelection();

[aelias_OOO_until_Jul13, 2017/02/10 21:43:55]

If text is inserted by typing on a physical keyboard or by Javascript method, it
doesn't go through this method (it's only for IME).  So this spot is too
high-level for this, it needs to be in lower-level code.

+
   int textStart = selectionRange.start();
 
   if (text.length()) {
     if (!insertText(text))
       return false;
 
     Element* rootEditableElement = frame().selection().rootEditableElement();
     if (rootEditableElement) {
       addCompositionUnderlines(underlines, rootEditableElement, textStart);
     }
@@ skipping 76 matching lines @@
 
   selectComposition();
 
   if (frame().selection().isNone())
     return;
 
   Element* target = document().focusedElement();
   if (!target)
     return;
 
+  clearSuggestionMarkersTouchingSelection();
+
   // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
   // needs to be audited. see http://crbug.com/590369 for more details.
   document().updateStyleAndLayoutIgnorePendingStylesheets();
 
   PlainTextRange selectedRange = createSelectionRangeForSetComposition(
       selectionStart, selectionEnd, text.length());
-
   // Dispatch an appropriate composition event to the focused node.
   // We check the composition status and choose an appropriate composition event
   // since this function is used for three purposes:
   // 1. Starting a new composition.
   //    Send a compositionstart and a compositionupdate event when this function
   //    creates a new composition node, i.e. !hasComposition() &&
   //    !text.isEmpty().
   //    Sending a compositionupdate event at this time ensures that at least one
   //    compositionupdate event is dispatched.
   // 2. Updating the existing composition node.
@@ skipping 74 matching lines @@
   // needs to be audited. see http://crbug.com/590369 for more details.
   document().updateStyleAndLayoutIgnorePendingStylesheets();
 
   // We shouldn't close typing in the middle of setComposition.
   setEditableSelectionOffsets(selectedRange, NotUserTriggered);
 
   if (underlines.isEmpty()) {
     document().markers().addCompositionMarker(
         m_compositionRange->startPosition(), m_compositionRange->endPosition(),
         Color::black, false,
-        LayoutTheme::theme().platformDefaultCompositionBackgroundColor());
+        LayoutTheme::theme().platformDefaultCompositionBackgroundColor(),
+        std::vector<std::string>());
     return;
   }
 
   addCompositionUnderlines(underlines, baseNode->parentNode(), baseOffset);
 }
 
 PlainTextRange InputMethodController::createSelectionRangeForSetComposition(
     int selectionStart,
     int selectionEnd,
     size_t textLength) const {
@@ skipping 446 matching lines @@
       return WebTextInputTypeDateTimeField;
   }
 
   document().updateStyleAndLayoutTree();
   if (hasEditableStyle(*element))
     return WebTextInputTypeContentEditable;
 
   return WebTextInputTypeNone;
 }
 
+WebVector<blink::WebTextSuggestionInfo>
+InputMethodController::getTextSuggestionInfosUnderCaret() const {

[aelias_OOO_until_Jul13, 2017/02/10 21:43:55]

Please move this and most other changes in this file into a new file called
something like MobileSuggestions.cpp

+  Element* rootEditableElement = frame().selection().rootEditableElement();
+  if (!rootEditableElement)
+    return WebVector<blink::WebTextSuggestionInfo>();
+
+  DocumentMarkerVector suggestionMarkers =
+      rootEditableElement->document().markers().markersInRangeInclusive(
+          firstEphemeralRangeOf(m_frame->selection().selection()),
+          DocumentMarker::Suggestion);
+
+  Vector<TextSuggestionList> suggestionSpans;
+  for (const Member<DocumentMarker>& marker : suggestionMarkers) {
+    // ignore ranges that have been collapsed as a result of editing
+    // operations
+    if (marker->endOffset() == marker->startOffset())
+      continue;
+
+    TextSuggestionList suggestionList;
+    suggestionList.documentMarkerID = marker->suggestionMarkerID();
+    suggestionList.start = marker->startOffset();
+    suggestionList.end = marker->endOffset();
+    suggestionList.suggestions = marker->suggestions();
+    suggestionSpans.push_back(suggestionList);
+  }
+
+  std::sort(suggestionSpans.begin(), suggestionSpans.end());
+
+  std::vector<WebTextSuggestionInfo> suggestionInfos;

[esprehn, 2017/01/31 22:41:34]

WTF Vector

[rlanday, 2017/01/31 23:30:09]

Ok

+
+  for (const TextSuggestionList& suggestionList : suggestionSpans) {
+    if (suggestionInfos.size() == kMaxNumberSuggestions)
+      break;
+
+    for (size_t suggestionIndex = 0;
+         suggestionIndex < suggestionList.suggestions.size();
+         suggestionIndex++) {
+      const std::string& suggestion =
+          suggestionList.suggestions[suggestionIndex];
+      bool isDupe = false;
+      for (size_t i = 0; i < suggestionInfos.size(); i++) {
+        const WebTextSuggestionInfo& otherSuggestionInfo = suggestionInfos[i];
+        if (otherSuggestionInfo.suggestion == suggestion) {
+          if (suggestionList.start == otherSuggestionInfo.spanStart &&
+              suggestionList.end == otherSuggestionInfo.spanEnd) {
+            isDupe = true;
+            break;
+          }
+        }
+      }
+
+      if (isDupe)
+        continue;
+
+      WebTextSuggestionInfo suggestionInfo;
+      suggestionInfo.documentMarkerID = suggestionList.documentMarkerID;
+      suggestionInfo.suggestionStart = 0;
+      suggestionInfo.suggestionEnd = suggestion.length();
+      suggestionInfo.spanStart = suggestionList.start;
+      suggestionInfo.spanEnd = suggestionList.end;
+      suggestionInfo.suggestionIndex = suggestionIndex;
+      suggestionInfo.suggestion = suggestion;
+      suggestionInfos.push_back(suggestionInfo);
+      if (suggestionInfos.size() == kMaxNumberSuggestions)
+        break;
+    }
+  }
+
+  if (suggestionInfos.size() == 0)
+    return WebVector<blink::WebTextSuggestionInfo>();
+
+  int spanUnionStart = suggestionInfos[0].spanStart;
+  int spanUnionEnd = suggestionInfos[0].spanEnd;
+
+  for (size_t i = 1; i < suggestionInfos.size(); i++) {
+    spanUnionStart = std::min(spanUnionStart, suggestionInfos[i].spanStart);
+    spanUnionEnd = std::max(spanUnionEnd, suggestionInfos[i].spanEnd);
+  }
+
+  for (WebTextSuggestionInfo& info : suggestionInfos) {
+    const EphemeralRange& prefixRange =
+        PlainTextRange(spanUnionStart, info.spanStart)
+            .createRange(*rootEditableElement);
+    String prefix =
+        plainText(prefixRange, TextIteratorBehavior::Builder().build());
+
+    const EphemeralRange& suffixRange =
+        PlainTextRange(info.spanEnd, spanUnionEnd)
+            .createRange(*rootEditableElement);
+    String suffix =
+        plainText(suffixRange, TextIteratorBehavior::Builder().build());
+
+    info.prefix = prefix.utf8().data();
+    info.suffix = suffix.utf8().data();
+  }
+
+  Color highlightColor;
+  if (suggestionMarkers[0]->underlineColor() != 0) {
+    highlightColor = suggestionMarkers[0]->underlineColor().combineWithAlpha(
+        kSuggestionUnderlineAlphaMultiplier);
+  } else {
+    highlightColor = LayoutTheme::tapHighlightColor();
+  }
+
+  EphemeralRange backgroundHighlightRange =
+      PlainTextRange(spanUnionStart, spanUnionEnd)
+          .createRange(*rootEditableElement);
+  document().markers().addSuggestionBackgroundHighlightMarker(

[aelias_OOO_until_Jul13, 2017/02/10 21:43:55]

I don't think a method called "get...()" should be adding a marker, this should
be split off.

+      backgroundHighlightRange.startPosition(),
+      backgroundHighlightRange.endPosition(), Color::black, false,
+      highlightColor);
+
+  return suggestionInfos;
+}
+
+void InputMethodController::prepareForTextSuggestionMenuToBeShown() {
+  frame().selection().setCaretVisible(false);
+}
+
 void InputMethodController::willChangeFocus() {
   if (!finishComposingText(DoNotKeepSelection))
     return;
   frame().chromeClient().resetInputMethod();
 }
 
 DEFINE_TRACE(InputMethodController) {
   visitor->trace(m_frame);
   visitor->trace(m_compositionRange);
   SynchronousMutationObserver::trace(visitor);
 }
 
 }  // namespace blink