See https://www.w3.org/TR/input-events/ and https://www.w3.org/TR/uievents/ for more details.
<rdar://problem/28681935>
Created attachment 291032 [details] Test patch (not for review)
Created attachment 291060 [details] First pass
Comment on attachment 291060 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=291060&action=review > Source/WebCore/dom/InputEvent.h:35 > struct InputEventInit : public UIEventInit { No need for "public" here. > Source/WebCore/dom/InputEvent.h:42 > + static Ref<InputEvent> create(const AtomicString& eventType, const String& inputType, bool canBubble, bool cancelable, DOMWindow* view, String data = String(), int detail = 0) Argument type should be const String& or perhaps String&&. Default can be { } rather than String(). But also, I am not sure we want default argument values here. > Source/WebCore/dom/InputEvent.h:52 > + InputEvent(const AtomicString& eventType, const String& inputType, bool canBubble, bool cancelable, DOMWindow*, String data, int detail); Argument type should be const String& or perhaps String&&. > Source/WebCore/dom/InputEvent.h:59 > String inputType() const { return m_inputType; } Return type should be const String&. > Source/WebCore/dom/InputEvent.h:60 > + String data() const { return m_data; } Return type should be const String&. > Source/WebCore/dom/Node.h:528 > + virtual void dispatchInputEvent(const String& inputType, String data = String()); This function should not be virtual. There are no overrides of it. As with most functions that dispatch an event to an event target, this function should not be a member function. Instead it should go where it is being used, perhaps in Editor.cpp, and likely the argument type can be something like Element& rather than Node&. The new argument to this function should be const String& rather than String. Or possibly String&& depending on how it is used. The default can be written { } rather than String(). That’s what we use in most new call sites like this one. > Source/WebCore/editing/CompositeEditCommand.h:118 > + virtual String inputEventData() const { return String(); } Can use { } rather than String() to return a null string. > Source/WebCore/editing/EditCommand.cpp:148 > + if (auto* frame = m_document->frame()) > + return frame->selection().selection().isInTextAreaOrTextInput(); > + > + return false; WebKit code calls for early return style, which means the error case is the return, not the main case. Or you could use &&: auto* frame = m_document->frame(); return frame && frame->selection().selection().isInTextAreaOrTextInput(); > Source/WebCore/editing/Editor.cpp:114 > +static bool dispatchBeforeInputEvent(Element& element, const AtomicString& inputType, String data = String()) The new argument to this function should be const String& rather than String. Or possibly String&& depending on how it is used. The default can be written { } rather than String(). That’s what we use in most new call sites like this one. > Source/WebCore/editing/Editor.cpp:1046 > +static bool dispatchBeforeInputEvents(RefPtr<Element> startRoot, RefPtr<Element> endRoot, const AtomicString& inputTypeName, String data = String()) The new argument to this function should be const String& rather than String. Or possibly String&& depending on how it is used. The default can be written { } rather than String(). That’s what we use in most new call sites like this one. > Source/WebCore/editing/Editor.cpp:1056 > +static void dispatchInputEvents(RefPtr<Element> startRoot, RefPtr<Element> endRoot, const AtomicString& inputTypeName, String data = String()) The new argument to this function should be const String& rather than String. Or possibly String&& depending on how it is used. The default can be written { } rather than String(). That’s what we use in most new call sites like this one. > Source/WebCore/editing/ReplaceRangeWithTextCommand.h:40 > + String inputEventData() const final; I suggest making this override private rather than putting it in the public. I don’t think anyone needs to call this non-polymorphically. > Source/WebCore/editing/SpellingCorrectionCommand.h:41 > + String inputEventData() const final; I suggest making this override private rather than putting it in the public. I don’t think anyone needs to call this non-polymorphically. > Source/WebCore/editing/VisibleSelection.cpp:671 > + Node* container = start().containerNode(); I suggest using auto* here. The type will be ContainerNode*, I think, not Node*, and that might even lead to slightly more efficient code. > Source/WebCore/editing/VisibleSelection.h:108 > + bool isInTextAreaOrTextInput() const; Can we put all of this logic into TypingCommand::isEditingTextAreaOrTextInput() and local helper functions if needed? We can always move the function here if someone else needs it, but I’d suggest keeping this concept all in one place for now.
Created attachment 291123 [details] Patch for landing
Comment on attachment 291123 [details] Patch for landing Clearing flags on attachment: 291123 Committed r207010: <http://trac.webkit.org/changeset/207010>
Comment on attachment 291123 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=291123&action=review > Source/WebCore/html/HTMLFormControlElement.cpp:329 > + HTMLElement::dispatchInputEvent(); This is a peculiar line of code. It should just call dispatchInputEvent, not HTMLElement::dispatchInputEvent, which is a peculiar way to write the same thing.
Comment on attachment 291123 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=291123&action=review >> Source/WebCore/html/HTMLFormControlElement.cpp:329 >> + HTMLElement::dispatchInputEvent(); > > This is a peculiar line of code. It should just call dispatchInputEvent, not HTMLElement::dispatchInputEvent, which is a peculiar way to write the same thing. Good catch. I will address this in a followup: https://bugs.webkit.org/show_bug.cgi?id=163236