[IME] Fire 'compositionend' after 'textInput' event and all other DOM updates

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 26 matching lines @@
 #include "core/frame/LocalFrame.h"
 #include "core/html/HTMLTextAreaElement.h"
 #include "core/input/EventHandler.h"
 #include "core/layout/LayoutObject.h"
 #include "core/layout/LayoutTheme.h"
 #include "core/page/ChromeClient.h"
 #include "wtf/Optional.h"
 
 namespace blink {
 
+namespace {
+
+void dispatchCompositionUpdateEvent(LocalFrame& frame, const String& text)
+{
+    Element* target = frame.document()->focusedElement();
+    if (!target)
+        return;
+
+    CompositionEvent* event = CompositionEvent::create(EventTypeNames::compositionupdate, frame.domWindow(), text);
+    target->dispatchEvent(event);
+}
+
+void dispatchCompositionEndEvent(LocalFrame& frame, const String& text)
+{
+    Element* target = frame.document()->focusedElement();
+    if (!target)
+        return;
+
+    CompositionEvent* event = CompositionEvent::create(EventTypeNames::compositionend, frame.domWindow(), text);
+    target->dispatchEvent(event);
+}
+
+// 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
+void insertTextDuringCompositionWithEvents(LocalFrame& frame, const String& text, TypingCommand::Options options, TypingCommand::TextCompositionType compositionType)
+{
+    CHECK(compositionType == TypingCommand::TextCompositionType::TextCompositionUpdate || compositionType == TypingCommand::TextCompositionType::TextCompositionConfirm);

[yosin_UTC9, 2016/05/20 07:58:12]

Please add "<< compositionType" for ease of debugging.

[chongz, 2016/05/20 17:44:48]

Done. Also changed to DCHECK().

+    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(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;

[chongz, 2016/05/19 21:44:19]

When would event handler destroy document...? I tested using Javascript but it
says I can only close window I created.

+
+    dispatchCompositionUpdateEvent(frame, text);
+    // 'compositionupdate' event handler may destroy document.
+    if (!frame.document())
+        return;
+
+    switch (compositionType) {
+    case TypingCommand::TextCompositionType::TextCompositionUpdate:
+        TypingCommand::insertText(*frame.document(), text, options, compositionType);
+        break;
+    case TypingCommand::TextCompositionType::TextCompositionConfirm:
+        // 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.
+}
+
+} // anonymous namespace
+
 InputMethodController::SelectionOffsetsScope::SelectionOffsetsScope(InputMethodController* inputMethodController)
     : m_inputMethodController(inputMethodController)
     , m_offsets(inputMethodController->getSelectionOffsets())
 {
 }
 
 InputMethodController::SelectionOffsetsScope::~SelectionOffsetsScope()
 {
     m_inputMethodController->setSelectionOffsets(m_offsets);
 }
@@ skipping 32 matching lines @@
     frame().document()->markers().removeMarkers(DocumentMarker::Composition);
     m_isDirty = false;
 }
 
 void InputMethodController::documentDetached()
 {
     clear();
     m_compositionRange = nullptr;
 }
 
-bool InputMethodController::insertTextForConfirmedComposition(const String& text)
-{
-    return frame().eventHandler().handleTextInputEvent(text, 0, TextEventInputComposition);
-}
-
 void InputMethodController::selectComposition() const
 {
     const EphemeralRange range = compositionEphemeralRange();
     if (range.isNull())
         return;
 
     // The composition can start inside a composed character sequence, so we have to override checks.
     // See <http://bugs.webkit.org/show_bug.cgi?id=15781>
     VisibleSelection selection;
     selection.setWithoutValidation(range.startPosition(), range.endPosition());
     frame().selection().setSelection(selection, 0);
 }
 
 bool InputMethodController::confirmComposition()
 {
     return confirmComposition(composingText());
 }
 
-static void dispatchCompositionEndEvent(LocalFrame& frame, const String& text)
-{
-    // We should send this event before sending a TextEvent as written in
-    // Section 6.2.2 and 6.2.3 of the DOM Event specification.
-    Element* target = frame.document()->focusedElement();
-    if (!target)
-        return;
-
-    CompositionEvent* event =
-        CompositionEvent::create(EventTypeNames::compositionend, frame.domWindow(), text);
-    target->dispatchEvent(event);
-}
-
 bool InputMethodController::confirmComposition(const String& text, ConfirmCompositionBehavior confirmBehavior)
 {
     if (!hasComposition())
         return false;
 
     Optional<Editor::RevealSelectionScope> revealSelectionScope;
     if (confirmBehavior == KeepSelection)
         revealSelectionScope.emplace(&editor());
 
     // If the composition was set from existing text and didn't change, then
     // there's nothing to do here (and we should avoid doing anything as that
     // may clobber multi-node styled text).
     if (!m_isDirty && composingText() == text) {
         clear();
         return true;
     }
 
     // Select the text that will be deleted or replaced.
     selectComposition();
 
     if (frame().selection().isNone())
         return false;
 
-    dispatchCompositionEndEvent(frame(), text);
-
     if (!frame().document())
         return false;
 
     // If text is empty, then delete the old composition here. If text is
     // non-empty, InsertTextCommand::input will delete the old composition with
     // an optimized replace operation.
     if (text.isEmpty())
         TypingCommand::deleteSelection(*frame().document(), 0);
 
     clear();
 
-    // TODO(chongz): DOM update should happen before 'compositionend' and along with 'compositionupdate'.
-    // https://crbug.com/575294
-    if (dispatchBeforeInputInsertText(frame().document()->focusedElement(), text) != DispatchEventResult::NotCanceled)
+    insertTextDuringCompositionWithEvents(frame(), text, 0, TypingCommand::TextCompositionType::TextCompositionConfirm);
+    // Event handler might destroy document.
+    if (!frame().document())
         return false;
 
-    insertTextForConfirmedComposition(text);
+    // No DOM update after 'compositionend'.
+    dispatchCompositionEndEvent(frame(), text);
 
     return true;
 }
 
 bool InputMethodController::confirmCompositionOrInsertText(const String& text, ConfirmCompositionBehavior confirmBehavior)
 {
     if (!hasComposition()) {
         if (!text.length())
             return false;
 
@@ skipping 19 matching lines @@
 void InputMethodController::cancelComposition()
 {
     if (!hasComposition())
         return;
 
     Editor::RevealSelectionScope revealSelectionScope(&editor());
 
     if (frame().selection().isNone())
         return;
 
-    dispatchCompositionEndEvent(frame(), emptyString());
     clear();
-    insertTextForConfirmedComposition(emptyString());
+
+    // TODO(chongz): Update InputType::DeleteComposedCharacter with latest discussion.
+    dispatchBeforeInputFromComposition(frame().document()->focusedElement(), InputEvent::InputType::DeleteComposedCharacter, emptyString(), InputEvent::EventCancelable::NotCancelable);
+    dispatchCompositionUpdateEvent(frame(), emptyString());
+    insertTextDuringCompositionWithEvents(frame(), emptyString(), 0, TypingCommand::TextCompositionType::TextCompositionConfirm);
+    // Event handler might destroy document.
+    if (!frame().document())
+        return;
 
     // An open typing command that disagrees about current selection would cause
     // issues with typing later on.
     TypingCommand::closeTyping(m_frame);
+
+    // No DOM update after 'compositionend'.
+    dispatchCompositionEndEvent(frame(), emptyString());
 }
 
 void InputMethodController::cancelCompositionIfSelectionIsInvalid()
 {
     if (!hasComposition() || editor().preventRevealSelection())
         return;
 
     // Check if selection start and selection end are valid.
     FrameSelection& selection = frame().selection();
     if (!selection.isNone() && !m_compositionRange->collapsed()) {
@@ skipping 13 matching lines @@
     // Updates styles before setting selection for composition to prevent
     // inserting the previous composition text into text nodes oddly.
     // See https://bugs.webkit.org/show_bug.cgi?id=46868
     frame().document()->updateStyleAndLayoutTree();
 
     selectComposition();
 
     if (frame().selection().isNone())
         return;
 
-    if (Element* target = frame().document()->focusedElement()) {
-        // Dispatch an appropriate composition event to the focused node.
-        // We check the composition status and choose an appropriate composition event since this
-        // function is used for three purposes:
-        // 1. Starting a new composition.
-        //    Send a compositionstart and a compositionupdate event when this function creates
-        //    a new composition node, i.e.
-        //    !hasComposition() && !text.isEmpty().
-        //    Sending a compositionupdate event at this time ensures that at least one
-        //    compositionupdate event is dispatched.
-        // 2. Updating the existing composition node.
-        //    Send a compositionupdate event when this function updates the existing composition
-        //    node, i.e. hasComposition() && !text.isEmpty().
-        // 3. Canceling the ongoing composition.
-        //    Send a compositionend event when function deletes the existing composition node, i.e.
-        //    !hasComposition() && test.isEmpty().
-        CompositionEvent* event = nullptr;
-        if (!hasComposition()) {
-            // We should send a compositionstart event only when the given text is not empty because this
-            // function doesn't create a composition node when the text is empty.
-            if (!text.isEmpty()) {
-                target->dispatchEvent(CompositionEvent::create(EventTypeNames::compositionstart, frame().domWindow(), frame().selectedText()));
-                event = CompositionEvent::create(EventTypeNames::compositionupdate, frame().domWindow(), text);
-            }
-        } else {
-            if (!text.isEmpty())
-                event = CompositionEvent::create(EventTypeNames::compositionupdate, frame().domWindow(), text);
-            else
-                event = CompositionEvent::create(EventTypeNames::compositionend, frame().domWindow(), text);
+    Element* target = frame().document()->focusedElement();
+    if (!target)
+        return;
+
+    // Dispatch an appropriate composition event to the focused node.
+    // We check the composition status and choose an appropriate composition event since this
+    // function is used for three purposes:
+    // 1. Starting a new composition.
+    //    Send a compositionstart and a compositionupdate event when this function creates
+    //    a new composition node, i.e.
+    //    !hasComposition() && !text.isEmpty().
+    //    Sending a compositionupdate event at this time ensures that at least one
+    //    compositionupdate event is dispatched.
+    // 2. Updating the existing composition node.
+    //    Send a compositionupdate event when this function updates the existing composition
+    //    node, i.e. hasComposition() && !text.isEmpty().
+    // 3. Canceling the ongoing composition.
+    //    Send a compositionend event when function deletes the existing composition node, i.e.
+    //    !hasComposition() && test.isEmpty().
+    if (text.isEmpty()) {
+        if (hasComposition()) {
+            confirmComposition(emptyString());
+            return;
         }
-        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);
-            target->dispatchEvent(event);
-        }
+        // It's weird to call |setComposition()| with empty text outside composition, however some IME
+        // (e.g. Japanese IBus-Anthy) did this, so we simply delete selection without sending extra events.
+        TypingCommand::deleteSelection(*frame().document(), TypingCommand::PreventSpellChecking);
+        return;
     }
 
-    // If text is empty, then delete the old composition here. If text is non-empty, InsertTextCommand::input
-    // will delete the old composition with an optimized replace operation.
-    if (text.isEmpty()) {
-        DCHECK(frame().document());
-        TypingCommand::deleteSelection(*frame().document(), TypingCommand::PreventSpellChecking);
+    // We should send a 'compositionstart' event only when the given text is not empty because this
+    // function doesn't create a composition node when the text is empty.
+    if (!hasComposition()) {
+        target->dispatchEvent(CompositionEvent::create(EventTypeNames::compositionstart, frame().domWindow(), frame().selectedText()));
+        if (!frame().document())
+            return;
     }
 
+    CHECK(!text.isEmpty());

[yosin_UTC9, 2016/05/20 07:58:12]

Do we really need to use |CHECK()|?

[chongz, 2016/05/20 17:44:48]

Probably overkill since I already filtered it out. I was trying to make it clear
that empty text means something else and shouldn't go into here.

[yosin_UTC9, 2016/05/23 04:57:28]

I supposed to use |DCHECK()| instead of |CHECK()|. Having |DCHECK()| makes debug
code slow, but it provide some information.

+    CHECK(frame().document());

[yosin_UTC9, 2016/05/20 07:58:12]

This CHECK() makes L364 redundant. Which one should we leave?

[chongz, 2016/05/20 17:44:48]

L364 was trying to handle the case you mentioned before, where document might
got destroyed by event handler. However I cannot reproduce it using
Javascript... (I'm using "myWindow.close()")

Can I have more information on how to reproduce that? Thanks! (Also see L96)

[yosin_UTC9, 2016/05/23 04:57:28]

ClusterFuzz will find test case. ;-)

+
     clear();
 
-    if (text.isEmpty())
+    insertTextDuringCompositionWithEvents(frame(), text, TypingCommand::SelectInsertedText | TypingCommand::PreventSpellChecking, TypingCommand::TextCompositionUpdate);
+    // Event handlers might destroy document.
+    if (!frame().document())
         return;
-    DCHECK(frame().document());
-    TypingCommand::insertText(*frame().document(), text, TypingCommand::SelectInsertedText | TypingCommand::PreventSpellChecking, TypingCommand::TextCompositionUpdate);
 
     // Find out what node has the composition now.
     Position base = mostForwardCaretPosition(frame().selection().base());
     Node* baseNode = base.anchorNode();
     if (!baseNode || !baseNode->isTextNode())
         return;
 
     Position extent = frame().selection().extent();
     Node* extentNode = extent.anchorNode();
     if (baseNode != extentNode)
@@ skipping 171 matching lines @@
     TypingCommand::deleteSelection(*frame().document());
 }
 
 DEFINE_TRACE(InputMethodController)
 {
     visitor->trace(m_frame);
     visitor->trace(m_compositionRange);
 }
 
 } // namespace blink