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

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

Issue 2372493002: Workaround for setComposition styling clobber (Closed)
Patch Set: Address changwan@'s review Created 4 years, 3 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 df935114b462563caaec2e0304e1baef383c096a..a83192a5f9fe37a7f15cb70a2184491faebcf274 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -338,6 +338,30 @@ void InputMethodController::cancelCompositionIfSelectionIsInvalid()
frame().chromeClient().didCancelCompositionOnSelectionChange();
}
+size_t InputMethodController::computeCommonGraphemeClusterPrefixLengthForSetComposition(const String& newText) const
+{
+ String oldText = composingText();
+ size_t oldLength = oldText.length();
+ size_t newLength = newText.length();
+ size_t commonPrefixLength = 0;
+
+ while (commonPrefixLength < oldLength && commonPrefixLength < newLength && oldText[commonPrefixLength] == newText[commonPrefixLength])
yosin_UTC9 2016/10/04 09:00:05 Could you move this part to another function, e.g.
yabinh 2016/10/11 01:42:19 Done.
+ commonPrefixLength++;
yosin_UTC9 2016/10/04 09:00:05 nit: We prefer |++commonPrefixLength|
yabinh 2016/10/11 01:42:22 Done.
+
+ // For multi-code text, we should adjust it for grapheme boundary.
+ Element* rootEditableElement = frame().selection().rootEditableElement();
+ if (!rootEditableElement)
+ return 0;
+ const EphemeralRange range = PlainTextRange(0, commonPrefixLength).createRange(*rootEditableElement);
yosin_UTC9 2016/10/04 09:00:05 nit: s/const EphemeralRange/const EphemeralRange&/
yabinh 2016/10/11 01:42:19 Done.
+ if (range.isNull())
+ return 0;
+ Position position = range.endPosition();
yosin_UTC9 2016/10/04 09:00:05 nit: s/Position/const Position&/
yabinh 2016/10/11 01:42:22 Done.
+ Position adjustedPosition = previousPositionOf(nextPositionOf(position, PositionMoveType::GraphemeCluster), PositionMoveType::GraphemeCluster);
yosin_UTC9 2016/10/04 09:00:05 nit: s/Position/const Position&/
yabinh 2016/10/11 01:42:19 Done.
+ int diff = position.computeOffsetInContainerNode() - adjustedPosition.computeOffsetInContainerNode();
yosin_UTC9 2016/10/04 09:00:05 We should support position.containerNode() != adju
yabinh 2016/10/11 01:42:19 It seems that we couldn't find such a case. As we
+
+ return commonPrefixLength - static_cast<size_t>(diff);
+}
+
void InputMethodController::setComposition(const String& text, const Vector<CompositionUnderline>& underlines, int selectionStart, int selectionEnd)
{
Editor::RevealSelectionScope revealSelectionScope(&editor());
@@ -347,6 +371,61 @@ void InputMethodController::setComposition(const String& text, const Vector<Comp
// 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()) {
+ size_t composingLength = composingText().length();
yosin_UTC9 2016/10/04 09:00:05 Please move this part to another function. This pa
yabinh 2016/10/11 01:42:22 Done.
+ size_t commonPrefixLength = computeCommonGraphemeClusterPrefixLengthForSetComposition(text);
+
+ bool appending = text.length() > commonPrefixLength;
yosin_UTC9 2016/10/04 09:00:05 nit: s/bool/const bool/
yabinh 2016/10/11 01:42:21 Done.
+ bool backspacing = composingLength > commonPrefixLength;
yosin_UTC9 2016/10/04 09:00:05 nit: s/bool/const bool/; since this variable used
yabinh 2016/10/11 01:42:20 Done.
+ if (appending || backspacing) {
+ const EphemeralRange range = compositionEphemeralRange();
yosin_UTC9 2016/10/04 09:00:05 s/const EphemeralRange/const EphemeralRange&/ Bet
yabinh 2016/10/11 01:42:21 Done.
+ Element* editable = frame().selection().rootEditableElement();
+ if (!editable)
+ return;
+
+ // Move selection to the end of the composition.
+ PlainTextRange compositionPlainOffsets = PlainTextRange::create(*editable, range);
+ VisibleSelection selection;
+ selection.setWithoutValidation(range.endPosition(), range.endPosition());
+ frame().selection().setSelection(selection, 0);
+ clear();
+
+ Element* target = frame().document()->focusedElement();
+ if (!target)
+ return;
+
+ // Apply the incremental change. Select the text to be deleted (if
+ // needed) and replace it with the incremental text (or empty text).
+ if (backspacing) {
+ PlainTextRange selectionOffsets(getSelectionOffsets());
+ size_t deletedLength = composingLength - commonPrefixLength;
+ setEditableSelectionOffsets(PlainTextRange(selectionOffsets.start() - deletedLength, selectionOffsets.end()), NotUserTriggered);
+ }
+ insertTextDuringCompositionWithEvents(frame(), text.substring(commonPrefixLength), TypingCommand::PreventSpellChecking, TypingCommand::TextCompositionUpdate);
+
+ // Event handlers might destroy document.
+ if (!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, compositionPlainOffsets.start(), compositionPlainOffsets.start() + text.length());
+ selectComposition();
+ PlainTextRange selectedRange = createSelectionRangeForSetComposition(selectionStart, selectionEnd, text.length());
yosin_UTC9 2016/10/04 09:00:05 nit: s/PalinTextRange/const PlainTextRange&/
yabinh 2016/10/11 01:42:22 Done.
+ // We shouldn't close typing in the middle of setComposition.
+ setEditableSelectionOffsets(selectedRange, NotUserTriggered);
+ m_isDirty = true;
+ return;
+ }
+ }
+
selectComposition();
if (frame().selection().isNone())
@@ -356,14 +435,7 @@ void InputMethodController::setComposition(const String& text, const Vector<Comp
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 since this
@@ -466,6 +538,18 @@ void InputMethodController::setComposition(const String& text, const Vector<Comp
}
}
+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());
+ int start = selectionOffsetsStart + selectionStart;
+ int end = selectionOffsetsStart + selectionEnd;
+ return createRangeForSelection(start, end, textLength);
+}
+
void InputMethodController::setCompositionFromExistingText(const Vector<CompositionUnderline>& underlines, unsigned compositionStart, unsigned compositionEnd)
{
Element* editable = frame().selection().rootEditableElement();

Powered by Google App Engine
This is Rietveld 408576698