Introduce InsertIncrementalTextCommand to respect existing style for composition

third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp

+// Copyright (c) 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/editing/commands/InsertIncrementalTextCommand.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/PlainTextRange.h"
+#include "core/editing/VisibleUnits.h"
+#include "core/frame/LocalFrame.h"
+#include "core/html/HTMLSpanElement.h"
+
+namespace blink {
+
+InsertIncrementalTextCommand::InsertIncrementalTextCommand(
+    Document& document,
+    const String& text,
+    bool selectInsertedText,
+    RebalanceType rebalanceType)
+    : InsertTextCommand(document, text, selectInsertedText, rebalanceType) {}
+
+static size_t computeCommonPrefixLength(const String& str1,

[yosin_UTC9, 2016/12/07 05:36:22]

Let's use unnamed namespace:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
instead of |static| to follow chromium C++ style.

[yabinh, 2016/12/08 07:54:57]

Done.

+                                        const String& str2) {
+  const size_t maxCommonPrefixLength = std::min(str1.length(), str2.length());
+  for (size_t index = 0; index < maxCommonPrefixLength; ++index) {
+    if (str1[index] != str2[index])
+      return index;
+  }
+  return maxCommonPrefixLength;
+}
+
+static size_t computeCommonSuffixLength(const String& str1,
+                                        const String& str2) {
+  const size_t length1 = str1.length();
+  const size_t length2 = str2.length();
+  const size_t maxCommonSuffixLength = std::min(length1, length2);
+  for (size_t index = 0; index < maxCommonSuffixLength; ++index) {
+    if (str1[length1 - index - 1] != str2[length2 - index - 1])
+      return index;
+  }
+  return maxCommonSuffixLength;
+}
+
+// If current position is at grapheme boundary, return 0; otherwise, return the
+// distance to its nearest left grapheme boundary.
+static size_t computeDistanceToLeftGraphemeBoundary(const Position& position) {
+  const Position& adjustedPosition = previousPositionOf(
+      nextPositionOf(position, PositionMoveType::GraphemeCluster),
+      PositionMoveType::GraphemeCluster);
+  DCHECK_EQ(position.anchorNode(), adjustedPosition.anchorNode());
+  DCHECK_GE(position.computeOffsetInContainerNode(),
+            adjustedPosition.computeOffsetInContainerNode());
+  return static_cast<size_t>(position.computeOffsetInContainerNode() -
+                             adjustedPosition.computeOffsetInContainerNode());
+}
+
+static size_t computeCommonGraphemeClusterPrefixLength(
+    const String& oldText,
+    const String& newText,
+    const Element* rootEditableElement) {
+  const size_t commonPrefixLength = computeCommonPrefixLength(oldText, newText);
+
+  // For grapheme cluster, we should adjust it for grapheme boundary.
+  const EphemeralRange& range =
+      PlainTextRange(0, commonPrefixLength).createRange(*rootEditableElement);

[Xiaocheng, 2016/12/07 10:23:26]

Is this correct when |oldText| is not at the beginning of |rootEditableElement|?

[yabinh, 2016/12/08 07:54:57]

Yes. See InputMethodControllerTest#SetCompositionKeepingStyle

+  if (range.isNull())
+    return 0;
+  const Position& position = range.endPosition();
+  const size_t diff = computeDistanceToLeftGraphemeBoundary(position);
+  DCHECK_GE(commonPrefixLength, diff);
+  return commonPrefixLength - diff;
+}
+
+// If current position is at grapheme boundary, return 0; otherwise, return the
+// distance to its nearest right grapheme boundary.
+static size_t computeDistanceToRightGraphemeBoundary(const Position& position) {
+  const Position& adjustedPosition = nextPositionOf(
+      previousPositionOf(position, PositionMoveType::GraphemeCluster),
+      PositionMoveType::GraphemeCluster);
+  DCHECK_EQ(position.anchorNode(), adjustedPosition.anchorNode());
+  DCHECK_GE(adjustedPosition.computeOffsetInContainerNode(),
+            position.computeOffsetInContainerNode());
+  return static_cast<size_t>(adjustedPosition.computeOffsetInContainerNode() -
+                             position.computeOffsetInContainerNode());
+}
+
+static size_t computeCommonGraphemeClusterSuffixLength(
+    const String& oldText,
+    const String& newText,
+    const Element* rootEditableElement) {
+  const size_t commonSuffixLength = computeCommonSuffixLength(oldText, newText);
+
+  // For grapheme cluster, we should adjust it for grapheme boundary.
+  const EphemeralRange& range =
+      PlainTextRange(0, oldText.length() - commonSuffixLength)

[Xiaocheng, 2016/12/07 10:23:26]

Is this correct when |oldText| is not at the end of |rootEditableElement|?

[yabinh, 2016/12/08 07:54:58]

ditto

+          .createRange(*rootEditableElement);
+  if (range.isNull())
+    return 0;
+  const Position& position = range.endPosition();
+  const size_t diff = computeDistanceToRightGraphemeBoundary(position);
+  DCHECK_GE(commonSuffixLength, diff);
+  return commonSuffixLength - diff;
+}
+
+static const String computeTextForInsertion(const String& newText,
+                                            const size_t commonPrefixLength,
+                                            const size_t commonSuffixLength) {
+  return newText.substring(
+      commonPrefixLength,
+      newText.length() - commonPrefixLength - commonSuffixLength);
+}
+
+static PlainTextRange getSelectionOffsets(LocalFrame* frame) {

[Xiaocheng, 2016/12/07 10:23:26]

Let's get rid of this function. See comments on doApply()

[yabinh, 2016/12/08 07:54:57]

Done.

+  EphemeralRange range = firstEphemeralRangeOf(frame->selection().selection());
+  if (range.isNull())
+    return PlainTextRange();
+  ContainerNode* editable =
+      frame->selection().rootEditableElementOrTreeScopeRootNode();
+  DCHECK(editable);
+
+  return PlainTextRange::create(*editable, range);
+}
+
+static const VisibleSelection createSelection(const size_t start,

[Xiaocheng, 2016/12/07 10:23:25]

Let's get rid of this function. See comments on doApply().

[yabinh, 2016/12/08 07:54:57]

Done.

+                                              const size_t end,
+                                              const bool isDirectional,
+                                              LocalFrame* frame) {
+  Element* element = frame->selection().selection().rootEditableElement();

[yosin_UTC9, 2016/12/07 05:23:34]

Make root editable as parameter rather than passing |LocalFrame*|.

[yabinh, 2016/12/08 07:54:58]

Done.

+  DCHECK(element);
+
+  const EphemeralRange& startRange =
+      PlainTextRange(0, static_cast<int>(start)).createRange(*element);
+  DCHECK(startRange.isNotNull());
+  const Position& startPosition = startRange.endPosition();
+
+  const EphemeralRange& endRange =
+      PlainTextRange(0, static_cast<int>(end)).createRange(*element);
+  DCHECK(endRange.isNotNull());
+  const Position& endPosition = endRange.endPosition();
+
+  VisibleSelection selection =

[yosin_UTC9, 2016/12/07 05:23:34]

nit: s/VisibleSelection/const VisibleSelection&/

[yabinh, 2016/12/08 07:54:58]

Done.

+      createVisibleSelection(SelectionInDOMTree::Builder()
+                                 .setBaseAndExtent(startPosition, endPosition)
+                                 .build());
+  selection.setIsDirectional(isDirectional);

[yosin_UTC9, 2016/12/07 05:23:34]

Use SelectionInDOMTree::Builder::setIsDirectional()

[yabinh, 2016/12/08 07:54:58]

Done.

+
+  return selection;
+}
+
+const VisibleSelection

[yosin_UTC9, 2016/12/07 05:23:34]

nit: s/const//

[yabinh, 2016/12/08 07:54:58]

This function has been removed.

+InsertIncrementalTextCommand::computeSelectionForInsertion(

[Xiaocheng, 2016/12/07 10:23:26]

Let's get rid of this function. See comments on doApply().

[yabinh, 2016/12/08 07:54:57]

Done.

+    const size_t selectionStart,
+    const size_t selectionEnd,
+    const size_t commonPrefixLength,
+    const size_t commonSuffixLength) {
+  const size_t insertionStart = selectionStart + commonPrefixLength;
+  const size_t insertionEnd = selectionEnd - commonSuffixLength;
+  DCHECK_LE(insertionStart, insertionEnd);
+
+  const VisibleSelection selectionForInsertion =

[yosin_UTC9, 2016/12/07 05:23:34]

nit: s/VisibleSelection/const VisibleSelection&/

[yabinh, 2016/12/08 07:54:57]

This function has been removed.

+      createSelection(insertionStart, insertionEnd,
+                      endingSelection().isDirectional(), document().frame());
+
+  return selectionForInsertion;
+}
+
+void InsertIncrementalTextCommand::setSelection(const size_t start,

[yosin_UTC9, 2016/12/07 05:23:34]

EditingCommand should not set selection by it self. Use endingSelection() to
work with undo/redo.

[yabinh, 2016/12/08 07:54:58]

Done.

+                                                const size_t end,
+                                                LocalFrame* frame) {
+  const VisibleSelection selection =
+      createSelection(start, end, endingSelection().isDirectional(), frame);
+  setStartingSelection(selection);

[Xiaocheng, 2016/12/07 10:23:25]

I don't think we should change the starting selection after the command
finishes. Setting ending selection seems enough.

[yabinh, 2016/12/08 07:54:58]

Done.

+  setEndingSelectionWithoutValidation(selection.start(), selection.end());
+
+  document().frame()->selection().setSelection(selection);
+}
+
+void InsertIncrementalTextCommand::doApply(EditingState* editingState) {
+  LocalFrame* frame = document().frame();
+  DCHECK(frame);
+  const Element* element = endingSelection().rootEditableElement();
+  DCHECK(element);
+
+  const String oldText = frame->selectedText();

[Xiaocheng, 2016/12/07 10:23:26]

Accessing FrameSelection from EditCommand is discouraged. Use the following
instead:

const EphemeralRange selectionRange(endingSelection().start(),
endingSelection().end());
oldText = plainText(selectionRange);

[yabinh, 2016/12/08 07:54:58]

Done.

+  const String& newText = m_text;
+  const size_t newTextLength = newText.length();
+  const size_t commonPrefixLength =
+      computeCommonGraphemeClusterPrefixLength(oldText, newText, element);
+  // We should ignore common prefix when finding common suffix.
+  const size_t commonSuffixLength = computeCommonGraphemeClusterSuffixLength(
+      oldText.right(oldText.length() - commonPrefixLength),
+      newText.right(newTextLength - commonPrefixLength), element);
+
+  m_text =
+      computeTextForInsertion(m_text, commonPrefixLength, commonSuffixLength);
+
+  const PlainTextRange selectionOffsets = getSelectionOffsets(frame);

[Xiaocheng, 2016/12/07 10:23:26]

Use CharacterIterator::calculateCharacterSubrange to prevent frequent
translation between offsets and Positions, and also avoid accessing
FrameSelection:

CharacterIterator charIt(selectionRange);
const EphemeralRange& rangeForInsertion =
charIt.calculateCharacterSubrange(commonPrefixLength, m_text.length());
const VisibleSelection& selectionForInsertion =
createVisibleSelection(SelectionInDOMTree::Builder().setBaseAndExtent(rangeForInsertion).setIsDirectional(endingSelection().isDirectional()).build());

[yabinh, 2016/12/08 07:54:58]

Done.

+  const size_t selectionStart = selectionOffsets.start();
+  const size_t selectionEnd = selectionOffsets.end();
+  const VisibleSelection selectionForInsertion = computeSelectionForInsertion(
+      selectionStart, selectionEnd, commonPrefixLength, commonSuffixLength);
+
+  const bool changeSelection = selectionForInsertion != endingSelection();
+
+  setStartingSelection(selectionForInsertion);

[Xiaocheng, 2016/12/07 10:23:26]

I don't think we should change starting selection. Is there any client of the
changed starting selection?

[yabinh, 2016/12/08 07:54:58]

Done.

+  setEndingSelectionWithoutValidation(selectionForInsertion.start(),
+                                      selectionForInsertion.end());
+
+  InsertTextCommand::doApply(editingState);
+
+  if (editingState->isAborted())
+    return;
+  if (changeSelection)
+    setSelection(selectionStart, selectionStart + newTextLength, frame);

[Xiaocheng, 2016/12/07 10:23:26]

Use |setEndingSelection| instead.

[yabinh, 2016/12/08 07:54:57]

This function has been removed.

+}
+
+}  // namespace blink