[InputEvent] Fire 'beforeinput' during typing, pressing hot keys and IME composition

third_party/WebKit/Source/core/editing/InputMethodController.cpp

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 e1d2f0fe18572803afa15c40bfe7af59a60e36ee..2f9b0a9d6f9273440ca3bcce839325b54851ef50 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -172,6 +172,12 @@ bool InputMethodController::confirmComposition(const String& text, ConfirmCompos
 
     clear();
 
+    // According to spec 'beforeinput' should not be sent along with  CompositionEnd, so

[ojan, 2016/04/16 00:11:45]

Nit: two spaces before CompositionEnd

[chongz, 2016/04/18 22:54:47]

Done.

+    // dispatch after CompositionEnd if there is any update.
+    // https://w3c.github.io/uievents/#h-events-composition-event-input-events

[ojan, 2016/04/16 00:11:45]

I believe the point of this part of the spec is that we currently have bugs
where we modify the content after compositionEnd. We should fix those bugs and
then we don't need to dispatch the event.

"Since there are no DOM updates associated with the compositionend event,
beforeinput and input events	should not be sent at that time."

The insertTextForConfirmedComposition line is actually a violation of the spec.
We either need to fix this or get the spec changed. I think we had decided in
January that if Chrome is able to change our behavior here that would be
preferable. Gary might be able to confirm.

In either case, for the purposes of this patch, I'm OK with this as is, but you
should file and bug to fix this and definitely have a TODO here.

[chongz, 2016/04/18 22:54:47]

Yes we shouldn't modify DOM after 'compositionend', will fix it.
Tracking bug https://crbug.com/575294

+    if (dispatchBeforeInputInsertText(frame().document()->focusedElement(), text) != DispatchEventResult::NotCanceled)
+        return false;
+
     insertTextForConfirmedComposition(text);
 
     return true;
@@ -182,6 +188,10 @@ bool InputMethodController::confirmCompositionOrInsertText(const String& text, C
     if (!hasComposition()) {
         if (!text.length())
             return false;
+
+        if (dispatchBeforeInputInsertText(frame().document()->focusedElement(), text) != DispatchEventResult::NotCanceled)
+            return false;
+
         editor().insertText(text, 0);
         return true;
     }
@@ -278,8 +288,13 @@ void InputMethodController::setComposition(const String& text, const Vector<Comp
             else
                 event = CompositionEvent::create(EventTypeNames::compositionend, frame().domWindow(), text);
         }
-        if (event)
+        if (event) {
+            // TODO(chongz): Support canceling IME composition.
+            // TODO(chongz): Should fire InsertText or DeleteComposedCharacter based on action.
+            if (event->type() == EventTypeNames::compositionupdate)
+                dispatchBeforeInputFromComposition(target, InputEvent::InputType::InsertText, text);

[chongz, 2016/04/13 23:53:09]

Based on discussion on GitHub DeleteComposedCharacter should be fired here. Will
figure out a way to do it.

             target->dispatchEvent(event);
+        }
     }
 
     // If text is empty, then delete the old composition here. If text is non-empty, InsertTextCommand::input
@@ -453,6 +468,8 @@ void InputMethodController::extendSelectionAndDelete(int before, int after)
             break;
         ++before;
     } while (frame().selection().start() == frame().selection().end() && before <= static_cast<int>(selectionOffsets.start()));
+    // TODO(chongz): According to spec |data| should be "forward" or "backward".

[ojan, 2016/04/16 00:11:45]

Shouldn't data be the text that's going to be deleted?

[chongz, 2016/04/18 22:54:47]

Spec says:
```
6. If the user indicated deletion of content that causes the caret to move using
a keyboard, speech, or similar method, the inputType must be deleteContent
    1. Set the value of data to be "forward" or "backward", depending on whether
the caret would move forward or backward in connection with the deletion of
content.
```
If that's actually a bug I can file one.
http://w3c.github.io/editing/input-events.html#widl-InputEvent-inputType

[ojan, 2016/04/20 16:58:57]

Whoops. I forgot to respond to this. That behavior seems crazy to me. data
should always be the thing getting inserted, not some magical field that
contains arbitrary values. So, IMO, the forward/backward part of this spec is
definitely something that should be changed. Instead of reusing the data field
for this purpose, there should just be two different edit types
(forwardDeleteContent and backwardDeleteContent or something like that).

I'm also unsure about the replaceContent value of data being a documentFragment.
It's weird for it to sometimes be a string and sometimes a documentFragment.
But, maybe we should run that by more people on our team first. This might be a
good thing to discuss with the architecture team
(platform-architecture-dev@chromium), and then we file a bug on the spec. 

+    dispatchBeforeInputEditorCommand(frame().document()->focusedElement(), InputEvent::InputType::DeleteContent);
     TypingCommand::deleteSelection(*frame().document());
 }