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

third_party/WebKit/Source/core/events/InputEvent.cpp

Index: third_party/WebKit/Source/core/events/InputEvent.cpp
diff --git a/third_party/WebKit/Source/core/events/InputEvent.cpp b/third_party/WebKit/Source/core/events/InputEvent.cpp
index f7eb07a65d040c9cb0071ccc8a29dc660983bf09..35ccafae25b049189f26c58030f467885570c785 100644
--- a/third_party/WebKit/Source/core/events/InputEvent.cpp
+++ b/third_party/WebKit/Source/core/events/InputEvent.cpp
@@ -8,6 +8,22 @@
 
 namespace blink {
 
+namespace {
+
+String InputTypeToString(InputEvent::InputType inputType)

[chongz, 2016/03/15 19:19:10]

Cannot do |InputType m_inputType| because I need to create InputEvent from
|commandName|, or should I enum all the possible command names?

[yosin_UTC9, 2016/03/16 01:46:51]

|inputType| doesn't hold a command name rather it holds a type of input the
spec. So, we can define it as |enum class|.

[yosin_UTC9, 2016/03/16 01:51:10]

|inputType| is still in discussion, https://github.com/w3c/editing/issues/110

[chongz, 2016/03/16 21:51:44]

Ok, I also found a list here and I'm not sure which one should I follow. 
https://github.com/w3c/editing/issues/79

Maybe I'll start with "insertText", "replaceContent", "deleteContent", "Undo",
"Redo" first?

To map a command to an InputType do you want me to
1. Set Fire/DoNotFire in EditorCommand like now and build a
commandName-to-InputType map in InputEvent.cpp?
Or 2. Just store InputType in EditorCommand to replace Fire/DoNotFire?

Thanks!  

[yosin_UTC9, 2016/03/17 01:52:23]

Make sense. Start with well defined one then expand in future.
Let's use |enum class InputType|, then having string representation in
|InputEvent| constructor.
See http://crbug.com/251275, Using strings for actions is error prone

Pseudo code can be:


static const char* const kInputTypeStrings[] = { "insertText", "replaceText",
... };
const auto& it = std::begin(kInputTypeStrings) + static_cast<size_t>(inputType);
if (it < std::begin(kInputTypeStrings) || it <= std::end(kInputString)) {
  ASSERT_NOT_REACHED();
  return "";
}
return *it;

[ojan, 2016/03/17 18:12:59]

Using strings for code logic is definitely bad and the bug you point to is
really bad! But that's not what this code is doing, right? The problem with
having the string representation in the InputEvent constructor is that it's
generated from IDL and you'd now need a custom constructor. This switch is
*only* used to fill a string field in the input argument to the constructor, so
I don't think that's problematic. It just happens to be that the constructor
takes a dictionary argument instead of a string directly.

That said, I'm missing why we need an enum or separate string map at all. Can we
use the command name from the CommandMap in EditorCommand? I think we should
make the command names in EditorCommand match the ones we need to expose to web
authors, otherwise we have to maintain two maps, which is error-prone.

As far as firing or not firing, isn't that already controlled by the
shouldFireInputEvent value in the CommandMap? In every case we would fire the
input event, we should also be firing the beforeInput event (the opposite isn't
always true because the author can preventDefault).

[ojan, 2016/03/17 18:21:10]

Oh, I see, you added shouldFireInputEvent. For now you have this only
dispatching beforeInput, do you plan to change the input event firing to use
this logic as well? I think it's important that the two use the same code so we
don't have subtle bugs of cases where they don't match.

[chongz, 2016/04/13 00:34:28]

Done.

+{
+    switch (inputType) {
+    case InputEvent::InputType::InsertText:
+        return "insertText";
+    case InputEvent::InputType::ReplaceText:
+        return "replaceText";
+    }
+    ASSERT_NOT_REACHED();
+    return "";
+}
+
+} // anonymous namespace
+
 InputEvent::InputEvent()
 {
 }
@@ -15,6 +31,49 @@ InputEvent::InputEvent()
 InputEvent::InputEvent(const AtomicString& type, const InputEventInit& initializer)
     : UIEvent(type, initializer)
 {
+    if (initializer.hasInputType())
+        m_inputType = initializer.inputType();
+    if (initializer.hasData())
+        m_data = initializer.data();
+}
+
+/* static */
+PassRefPtrWillBeRawPtr<InputEvent> InputEvent::createBeforeInputTyping(InputType inputType, const String& data)
+{
+    InputEventInit inputEventInit;
+
+    inputEventInit.setBubbles(true);
+    inputEventInit.setCancelable(true);
+    inputEventInit.setInputType(InputTypeToString(inputType));
+    inputEventInit.setData(data);
+
+    return InputEvent::create(EventTypeNames::beforeinput, inputEventInit);
+}
+
+/* static */
+PassRefPtrWillBeRawPtr<InputEvent> InputEvent::createBeforeInputCompositionUpdate(InputType inputType, const String& data)
+{
+    InputEventInit inputEventInit;
+
+    inputEventInit.setBubbles(true);
+    inputEventInit.setCancelable(false);
+    inputEventInit.setInputType(InputTypeToString(inputType));
+    inputEventInit.setData(data);
+
+    return InputEvent::create(EventTypeNames::beforeinput, inputEventInit);
+}
+
+/* static */
+PassRefPtrWillBeRawPtr<InputEvent> InputEvent::createBeforeInputEditorCommand(const String& commandName, const String& data)
+{
+    InputEventInit inputEventInit;
+
+    inputEventInit.setBubbles(true);
+    inputEventInit.setCancelable(true);
+    inputEventInit.setInputType(commandName);
+    inputEventInit.setData(data);
+
+    return InputEvent::create(EventTypeNames::beforeinput, inputEventInit);
 }
 
 bool InputEvent::isInputEvent() const