[InputEvent] Move 'beforeinput' logic into |CompositeEditCommand::willApplyEditing()| (3/3)

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 66 matching lines @@
   // replaced) or the new text (text to be inserted) is empty.
   if (frame.selectedText().isEmpty() || newText.isEmpty())
     return false;
 
   return true;
 }
 
 // Used to insert/replace text during composition update and confirm
 // composition.
 // Procedure:
-//   1. Fire 'beforeinput' event for (TODO(chongz): deleted composed text) and
-//      inserted text
-//   2. Fire 'compositionupdate' event
-//   3. Fire TextEvent and modify DOM
-//   TODO(chongz): 4. Fire 'input' event
+//   1. Fire 'compositionupdate' event

[Xiaocheng, 2016/12/21 02:56:39]

Please revise the comments here.

[chongz, 2016/12/21 23:59:24]

Reverted comments since there is no behavior change.

+//   2. Use TextEvent or TypingCommand to modify DOM, which will fire
+//      'beforeinput' and 'input'
 void insertTextDuringCompositionWithEvents(
     LocalFrame& frame,
     const String& text,
     TypingCommand::Options options,
     TypingCommand::TextCompositionType compositionType) {
   DCHECK(compositionType ==
              TypingCommand::TextCompositionType::TextCompositionUpdate ||
          compositionType ==
              TypingCommand::TextCompositionType::TextCompositionConfirm ||
          compositionType ==
              TypingCommand::TextCompositionType::TextCompositionCancel)
       << "compositionType should be TextCompositionUpdate or "
          "TextCompositionConfirm  or TextCompositionCancel, but got "
       << static_cast<int>(compositionType);
   if (!frame.document())
     return;
 
-  Element* target = frame.document()->focusedElement();
-  if (!target)
-    return;
-
-  // TODO(chongz): Fire 'beforeinput' for the composed text being
-  // replaced/deleted.
-
-  // Only the last confirmed text is cancelable.
-  InputEvent::EventCancelable beforeInputCancelable =
-      (compositionType ==
-       TypingCommand::TextCompositionType::TextCompositionUpdate)
-          ? InputEvent::EventCancelable::NotCancelable
-          : InputEvent::EventCancelable::IsCancelable;
-  DispatchEventResult result = dispatchBeforeInputFromComposition(

[chongz, 2016/12/20 23:27:52]

Kept, still fire 'beforeinput' before 'compositionupdate'.

-      target, InputEvent::InputType::InsertText, text, beforeInputCancelable);
-
-  if (beforeInputCancelable == InputEvent::EventCancelable::IsCancelable &&
-      result != DispatchEventResult::NotCanceled)
-    return;
-
-  // 'beforeinput' event handler may destroy document.
-  if (!frame.document())
-    return;
+  // TODO(chongz): Remove the following code after we have ensured it's OK to
+  // fire 'beforeinput' after 'compositionupdate'.
+  // https://crbug.com/675820
+  if (compositionType == TypingCommand::TextCompositionUpdate ||
+      compositionType == TypingCommand::TextCompositionConfirm) {
+    Element* target = frame.document()->focusedElement();
+    if (!target)
+      return;
+    // Only the last confirmed text is cancelable.
+    const InputEvent::EventCancelable beforeInputCancelable =
+        (compositionType == TypingCommand::TextCompositionUpdate)
+            ? InputEvent::EventCancelable::NotCancelable
+            : InputEvent::EventCancelable::IsCancelable;
+    RangeVector* targetRanges = nullptr;
+    if (frame.editor().canEditRichly())
+      targetRanges = new RangeVector(1, frame.selection().firstRange());
+    const bool isCanceled = !dispatchBeforeInputEvent(
+        EditCommandSource::kMenuOrKeyBinding, &frame, target,
+        InputEvent::InputType::InsertText, text, nullptr, beforeInputCancelable,
+        InputEvent::IsComposing, targetRanges);
+    if (isCanceled)
+      return;
+  }
 
   dispatchCompositionUpdateEvent(frame, text);
   // 'compositionupdate' event handler may destroy document.
-  if (!frame.document())
+  if (!frame.document() || frame.document()->frame() != &frame)

[Xiaocheng, 2016/12/21 02:56:38]

This seems irrelevant to beforeinput.

If it's a bug fix, could you isolate it out into another patch?

[chongz, 2016/12/21 23:59:24]

Reverted, will put in another patch.

     return;
 
   // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
   // needs to be audited. see http://crbug.com/590369 for more details.
   frame.document()->updateStyleAndLayoutIgnorePendingStylesheets();
 
   const bool isIncrementalInsertion = needsIncrementalInsertion(frame, text);
 
   switch (compositionType) {
     case TypingCommand::TextCompositionType::TextCompositionUpdate:
     case TypingCommand::TextCompositionType::TextCompositionConfirm:
       TypingCommand::insertText(
           *frame.document(), EditCommandSource::kMenuOrKeyBinding, text,
           options, compositionType, isIncrementalInsertion);
       break;
     case TypingCommand::TextCompositionType::TextCompositionCancel:
       // TODO(chongz): Use TypingCommand::insertText after TextEvent was
       // removed. (Removed from spec since 2012)
       // See TextEvent.idl.
       frame.eventHandler().handleTextInputEvent(text, 0,
                                                 TextEventInputComposition);
       break;
     default:
       NOTREACHED();
   }
-  // TODO(chongz): Fire 'input' event.

[Xiaocheng, 2016/12/21 02:56:39]

Is it a drive-by removal of a stale TODO?

If so, could you isolate it out into another patch?

[chongz, 2016/12/21 23:59:24]

Yes, will isolate into another patch.

 }
 
 AtomicString getInputModeAttribute(Element* element) {
   if (!element)
     return AtomicString();
 
   bool queryAttribute = false;
   if (isHTMLInputElement(*element)) {
     queryAttribute = toHTMLInputElement(*element).supportsInputModeAttribute();
   } else if (isHTMLTextAreaElement(*element)) {
@@ skipping 173 matching lines @@
   if (!moveCaret(absoluteCaretPosition))
     return false;
 
   // No DOM update after 'compositionend'.
   dispatchCompositionEndEvent(frame(), text);
 
   return true;
 }
 
 bool InputMethodController::insertText(const String& text) {
-  if (dispatchBeforeInputInsertText(document().focusedElement(), text) !=

[chongz, 2016/12/20 23:27:52]

Removed, inserting text was covered by |TypingCommand => willApplyEditing() ||
willAddTypingToOpenCommand()|.

-      DispatchEventResult::NotCanceled)
-    return false;
   editor().insertText(text, 0);
   return true;
 }
 
 bool InputMethodController::insertTextAndMoveCaret(const String& text,
                                                    int relativeCaretPosition) {
   PlainTextRange selectionRange = getSelectionOffsets();
   if (selectionRange.isNull())
     return false;
   int textStart = selectionRange.start();
@@ skipping 12 matching lines @@
   if (!hasComposition())
     return;
 
   Editor::RevealSelectionScope revealSelectionScope(&editor());
 
   if (frame().selection().isNone())
     return;
 
   clear();
 
-  // TODO(chongz): Figure out which InputType should we use here.
-  dispatchBeforeInputFromComposition(

[chongz, 2016/12/20 23:27:52]

Removed. This is a dummy 'beforeinput' with no actual editing behavior and
doesn't follow spec, will do further work on IME later.

-      document().focusedElement(),
-      InputEvent::InputType::DeleteComposedCharacterBackward, nullAtom,
-      InputEvent::EventCancelable::NotCancelable);
   dispatchCompositionUpdateEvent(frame(), emptyString());
   insertTextDuringCompositionWithEvents(
       frame(), emptyString(), 0,
       TypingCommand::TextCompositionType::TextCompositionCancel);
   // Event handler might destroy document.
   if (!isAvailable())
     return;
 
   // An open typing command that disagrees about current selection would cause
   // issues with typing later on.
@@ skipping 364 matching lines @@
   do {
     if (!setSelectionOffsets(PlainTextRange(
             std::max(static_cast<int>(selectionOffsets.start()) - before, 0),
             selectionOffsets.end() + after)))
       return;
     if (before == 0)
       break;
     ++before;
   } while (frame().selection().start() == frame().selection().end() &&
            before <= static_cast<int>(selectionOffsets.start()));
-  // TODO(chongz): Find a way to distinguish Forward and Backward.
-  dispatchBeforeInputEditorCommand(

[chongz, 2016/12/20 23:27:52]

Removed, deleting text was covered by |TypingCommand => willApplyEditing() ||
willAddTypingToOpenCommand()|.

-      document().focusedElement(), InputEvent::InputType::DeleteContentBackward,
-      new RangeVector(1, m_frame->selection().firstRange()));
   TypingCommand::deleteSelection(document(),
                                  EditCommandSource::kMenuOrKeyBinding);
 }
 
 // TODO(yabinh): We should reduce the number of selectionchange events.
 void InputMethodController::deleteSurroundingText(int before, int after) {
   if (!editor().canEdit())
     return;
   const PlainTextRange selectionOffsets(getSelectionOffsets());
   if (selectionOffsets.isNull())
@@ skipping 270 matching lines @@
   frame().chromeClient().resetInputMethod();
 }
 
 DEFINE_TRACE(InputMethodController) {
   visitor->trace(m_frame);
   visitor->trace(m_compositionRange);
   SynchronousMutationObserver::trace(visitor);
 }
 
 }  // namespace blink