Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(193)

Unified Diff: third_party/WebKit/Source/core/editing/InputMethodController.cpp

Issue 2372493002: Workaround for setComposition styling clobber (Closed)
Patch Set: Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 f8628dc7e842b61d332d1ad4b0a6870b9b25a253..00085d35b2e6c1029d143d1f4a11b2454bc419a2 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -357,6 +357,165 @@ void InputMethodController::cancelCompositionIfSelectionIsInvalid() {
frame().chromeClient().didCancelCompositionOnSelectionChange();
}
+static size_t computeCommonPrefixLength(const String& str1,
+ const String& str2) {
+ const size_t maxCommonPrefixLength = std::min(str1.length(), str2.length());
+ size_t commonPrefixLength;
+
+ for (commonPrefixLength = 0; commonPrefixLength < maxCommonPrefixLength;
yosin_UTC9 2016/10/11 08:32:41 Let's make scope of |commonPrefixLength| narrow.
yabinh 2016/10/11 08:55:01 Done.
+ ++commonPrefixLength) {
+ if (str1[commonPrefixLength] != str2[commonPrefixLength])
+ break;
+ }
+
+ return commonPrefixLength;
+}
+
+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);
+ size_t commonSuffixLength;
+
+ for (commonSuffixLength = 0; commonSuffixLength < maxCommonSuffixLength;
yosin_UTC9 2016/10/11 08:32:41 Ditto as computeCommonPrefixLength
yabinh 2016/10/11 08:55:01 Done.
+ ++commonSuffixLength) {
+ if (str1[length1 - commonSuffixLength - 1] !=
+ str2[length2 - commonSuffixLength - 1])
+ break;
+ }
+
+ return commonSuffixLength;
+}
+
+static size_t computeCommonGraphemeClusterPrefixLengthForSetComposition(
+ 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);
+ if (range.isNull())
+ return 0;
+ const Position& position = range.endPosition();
+ const Position& adjustedPosition = previousPositionOf(
+ nextPositionOf(position, PositionMoveType::GraphemeCluster),
+ PositionMoveType::GraphemeCluster);
+ DCHECK_EQ(position.anchorNode(), adjustedPosition.anchorNode());
+
+ int diff = position.computeOffsetInContainerNode() -
yosin_UTC9 2016/10/11 08:32:41 s/int/const int/
yabinh 2016/10/11 08:55:01 Done.
+ adjustedPosition.computeOffsetInContainerNode();
+ DCHECK_GE(diff, 0);
+
+ return commonPrefixLength - static_cast<size_t>(diff);
+}
+
+static size_t computeCommonGraphemeClusterSuffixLengthForSetComposition(
+ 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)
+ .createRange(*rootEditableElement);
+ if (range.isNull())
+ return 0;
+ const Position& position = range.endPosition();
+ const Position& adjustedPosition = nextPositionOf(
+ previousPositionOf(position, PositionMoveType::GraphemeCluster),
+ PositionMoveType::GraphemeCluster);
+ DCHECK_EQ(position.anchorNode(), adjustedPosition.anchorNode());
+
+ int diff = adjustedPosition.computeOffsetInContainerNode() -
yosin_UTC9 2016/10/11 08:32:41 s/int/const int/
yabinh 2016/10/11 08:55:01 Done.
+ position.computeOffsetInContainerNode();
+ DCHECK_GE(diff, 0);
+
yosin_UTC9 2016/10/11 08:32:41 nit: no need to have an extra blank line.
yabinh 2016/10/11 08:55:01 Done.
+ return commonSuffixLength - static_cast<size_t>(diff);
+}
+
+void InputMethodController::setCompositionWithIncrementalText(
+ const String& text,
+ const Vector<CompositionUnderline>& underlines,
+ int selectionStart,
+ int selectionEnd) {
+ Element* editable = frame().selection().rootEditableElement();
+ if (!editable)
+ return;
+
+ String composing = composingText();
+ const size_t commonPrefixLength =
+ computeCommonGraphemeClusterPrefixLengthForSetComposition(composing, text,
+ editable);
+
+ // We should ignore common prefix when finding common suffix.
+ const size_t commonSuffixLength =
+ computeCommonGraphemeClusterSuffixLengthForSetComposition(
+ composing.right(composing.length() - commonPrefixLength),
+ text.right(text.length() - commonPrefixLength), editable);
+
+ const bool appending =
+ text.length() > commonPrefixLength + commonSuffixLength;
+ const bool subtracting =
+ composing.length() > commonPrefixLength + +commonSuffixLength;
+
+ if (appending || subtracting) {
+ // Select the text to be subtracted.
+ size_t compositionStart =
yosin_UTC9 2016/10/11 08:32:41 nit: s/size_t/const size_t/
yabinh 2016/10/11 08:55:01 Done.
+ PlainTextRange::create(*editable, compositionEphemeralRange()).start();
+ size_t subtractionStart = compositionStart + commonPrefixLength;
yosin_UTC9 2016/10/11 08:32:41 nit: s/size_t/const size_t/
yabinh 2016/10/11 08:55:01 Done.
+ size_t subtractionEnd =
yosin_UTC9 2016/10/11 08:32:41 nit: s/size_t/const size_t/
yabinh 2016/10/11 08:55:01 Done.
+ compositionStart + composing.length() - commonSuffixLength;
+ const EphemeralRange& subtractionRange =
+ PlainTextRange(subtractionStart, subtractionEnd).createRange(*editable);
+ VisibleSelection selection;
+ selection.setWithoutValidation(subtractionRange.startPosition(),
+ subtractionRange.endPosition());
+ const Document* currentDocument = frame().document();
+ frame().selection().setSelection(selection, 0);
+ clear();
+
+ // FrameSeleciton::setSelection() can change document associate to |frame|.
+ if (*currentDocument != *frame().document())
+ return;
+ if (!currentDocument->focusedElement())
+ return;
+
+ // Append the incremental text.
+ size_t appendingLength =
+ text.length() - commonPrefixLength - commonSuffixLength;
+ String appendingText = text.substring(commonPrefixLength, appendingLength);
+ insertTextDuringCompositionWithEvents(frame(), appendingText,
+ TypingCommand::PreventSpellChecking,
+ TypingCommand::TextCompositionUpdate);
+
+ // Event handlers might destroy document.
+ if (!frame().document())
+ return;
+ if (*currentDocument != *frame().document())
+ return;
+
+ // TODO(yosin): The use of updateStyleAndLayoutIgnorePendingStylesheets
+ // needs to be audited. see http://crbug.com/590369 for more details.
+ frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
+
+ // Now recreate the composition starting at its original start, and
+ // apply the specified final selection offsets.
+ setCompositionFromExistingText(underlines, compositionStart,
+ compositionStart + text.length());
+ }
+
+ selectComposition();
+ const PlainTextRange& selectedRange = createSelectionRangeForSetComposition(
+ selectionStart, selectionEnd, text.length());
+ // We shouldn't close typing in the middle of setComposition.
+ setEditableSelectionOffsets(selectedRange, NotUserTriggered);
+ m_isDirty = true;
+}
+
void InputMethodController::setComposition(
const String& text,
const Vector<CompositionUnderline>& underlines,
@@ -369,6 +528,14 @@ void InputMethodController::setComposition(
// See https://bugs.webkit.org/show_bug.cgi?id=46868
frame().document()->updateStyleAndLayoutTree();
+ // When the IME only wants to change a few characters at the end of the
+ // composition, only touch those characters in order to preserve rich text
+ // substructure.
+ if (hasComposition() && text.length()) {
+ return setCompositionWithIncrementalText(text, underlines, selectionStart,
+ selectionEnd);
+ }
+
selectComposition();
if (frame().selection().isNone())
@@ -378,15 +545,8 @@ void InputMethodController::setComposition(
if (!target)
return;
- // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
- // needs to be audited. see http://crbug.com/590369 for more details.
- frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
-
- int selectionOffsetsStart = static_cast<int>(getSelectionOffsets().start());
- int start = selectionOffsetsStart + selectionStart;
- int end = selectionOffsetsStart + selectionEnd;
- PlainTextRange selectedRange =
- createRangeForSelection(start, end, text.length());
+ 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
@@ -503,6 +663,20 @@ void InputMethodController::setComposition(
}
}
+PlainTextRange InputMethodController::createSelectionRangeForSetComposition(
+ int selectionStart,
+ int selectionEnd,
+ size_t textLength) const {
+ // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
+ // needs to be audited. see http://crbug.com/590369 for more details.
+ frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
+
+ int selectionOffsetsStart = static_cast<int>(getSelectionOffsets().start());
yosin_UTC9 2016/10/11 08:32:41 nit: s/int/const int/
yabinh 2016/10/11 08:55:01 Done.
+ int start = selectionOffsetsStart + selectionStart;
+ int end = selectionOffsetsStart + selectionEnd;
+ return createRangeForSelection(start, end, textLength);
+}
+
void InputMethodController::setCompositionFromExistingText(
const Vector<CompositionUnderline>& underlines,
unsigned compositionStart,

Powered by Google App Engine
This is Rietveld 408576698