Bug 1338451 Editor should commit composition first at handling mousedown event r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 10 Feb 2017 16:51:59 +0900
changeset 484257 0d9f4a3779b04ef132f19aa7e86edf9af7b16887
parent 484187 1060668405a9399774c205430de4a7001d3f27ac
child 545749 78648eb6739f2a088ad52fefeabdd58611ab77c3
push id45433
push usermasayuki@d-toybox.com
push dateWed, 15 Feb 2017 02:54:24 +0000
reviewersm_kato
bugs1338451
milestone54.0a1
Bug 1338451 Editor should commit composition first at handling mousedown event r?m_kato Editor may need to create some new transaction during handling a mousedown event. Therefore, editor needs to finish pending composition transaction. So, mousedown event handler should commit composition first. MozReview-Commit-ID: 2BaoYwB7wLS
editor/libeditor/EditorEventListener.cpp
editor/libeditor/EditorEventListener.h
editor/libeditor/HTMLEditorEventListener.cpp
--- a/editor/libeditor/EditorEventListener.cpp
+++ b/editor/libeditor/EditorEventListener.cpp
@@ -352,16 +352,25 @@ EditorEventListener::DetachedFromEditor(
 bool
 EditorEventListener::DetachedFromEditorOrDefaultPrevented(
                        WidgetEvent* aWidgetEvent) const
 {
   return NS_WARN_IF(!aWidgetEvent) || DetachedFromEditor() ||
          aWidgetEvent->DefaultPrevented();
 }
 
+bool
+EditorEventListener::EnsureCommitCompoisition()
+{
+  MOZ_ASSERT(!DetachedFromEditor());
+  RefPtr<EditorBase> editorBase(mEditorBase);
+  editorBase->ForceCompositionEnd();
+  return !DetachedFromEditor();
+}
+
 NS_IMETHODIMP
 EditorEventListener::HandleEvent(nsIDOMEvent* aEvent)
 {
   // Let's handle each event with the message of the internal event of the
   // coming event.  If the DOM event was created with improper interface,
   // e.g., keydown event is created with |new MouseEvent("keydown", {});|,
   // its message is always 0.  Therefore, we can ban such strange event easy.
   // However, we need to handle strange "focus" and "blur" event.  See the
@@ -661,18 +670,17 @@ EditorEventListener::MouseClick(nsIDOMMo
 
   if (DetachedFromEditorOrDefaultPrevented(clickEvent)) {
     // We're done if 'preventdefault' is true (see for example bug 70698).
     return NS_OK;
   }
 
   // If we got a mouse down inside the editing area, we should force the
   // IME to commit before we change the cursor position
-  editorBase->ForceCompositionEnd();
-  if (DetachedFromEditor()) {
+  if (!EnsureCommitCompoisition()) {
     return NS_OK;
   }
 
   if (clickEvent->button == 1) {
     return HandleMiddleClickPaste(aMouseEvent);
   }
   return NS_OK;
 }
@@ -763,18 +771,17 @@ EditorEventListener::MouseDown(nsIDOMMou
 {
   // FYI: We don't need to check if it's already consumed here because
   //      we need to commit composition at mouse button operation.
   // FYI: This may be called by HTMLEditorEventListener::MouseDown() even
   //      when the event is not acceptable for committing composition.
   if (DetachedFromEditor()) {
     return NS_OK;
   }
-  RefPtr<EditorBase> editorBase(mEditorBase);
-  editorBase->ForceCompositionEnd();
+  Unused << EnsureCommitCompoisition();
   return NS_OK;
 }
 
 nsresult
 EditorEventListener::HandleChangeComposition(
                        WidgetCompositionEvent* aCompositionChangeEvent)
 {
   MOZ_ASSERT(!aCompositionChangeEvent->DefaultPrevented(),
--- a/editor/libeditor/EditorEventListener.h
+++ b/editor/libeditor/EditorEventListener.h
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef EditorEventListener_h
 #define EditorEventListener_h
 
+#include "mozilla/Attributes.h"
 #include "mozilla/EventForwards.h"
 #include "nsCOMPtr.h"
 #include "nsError.h"
 #include "nsIDOMEventListener.h"
 #include "nsISupportsImpl.h"
 #include "nscore.h"
 
 class nsCaret;
@@ -94,16 +95,23 @@ protected:
 
   /**
    * DetachedFromEditorOrDefaultPrevented() returns true if editor was detached
    * and/or the event was consumed.  Otherwise, i.e., attached editor can
    * handle the event, returns true.
    */
   bool DetachedFromEditorOrDefaultPrevented(WidgetEvent* aEvent) const;
 
+  /**
+   * EnsureCommitComposition() requests to commit composition if there is.
+   * Returns false if the editor is detached from the listener, i.e.,
+   * impossible to continue to handle the event.  Otherwise, true.
+   */
+  MOZ_MUST_USE bool EnsureCommitCompoisition();
+
   EditorBase* mEditorBase; // weak
   RefPtr<nsCaret> mCaret;
   bool mCommitText;
   bool mInTransaction;
   bool mMouseDownOrUpConsumedByIME;
 #ifdef HANDLE_NATIVE_TEXT_DIRECTION_SWITCH
   bool mHaveBidiKeyboards;
   bool mShouldSwitchTextDirection;
--- a/editor/libeditor/HTMLEditorEventListener.cpp
+++ b/editor/libeditor/HTMLEditorEventListener.cpp
@@ -77,33 +77,34 @@ HTMLEditorEventListener::MouseUp(nsIDOMM
 
 nsresult
 HTMLEditorEventListener::MouseDown(nsIDOMMouseEvent* aMouseEvent)
 {
   if (NS_WARN_IF(!aMouseEvent) || DetachedFromEditor()) {
     return NS_OK;
   }
 
+  // Even if it's not acceptable mousedown event (i.e., when mousedown
+  // event is fired outside of the active editing host), we need to commit
+  // composition because it will be change the selection to the clicked
+  // point.  Then, we won't be able to commit the composition.
+  if (!EnsureCommitCompoisition()) {
+    return NS_OK;
+  }
+
   WidgetMouseEvent* mousedownEvent =
     aMouseEvent->AsEvent()->WidgetEventPtr()->AsMouseEvent();
 
   HTMLEditor* htmlEditor = GetHTMLEditor();
   // Contenteditable should disregard mousedowns outside it.
   // IsAcceptableInputEvent() checks it for a mouse event.
   if (!htmlEditor->IsAcceptableInputEvent(mousedownEvent)) {
-    // If it's not acceptable mousedown event (including when mousedown event
-    // is fired outside of the active editing host), we need to commit
-    // composition because it will be change the selection to the clicked
-    // point.  Then, we won't be able to commit the composition.
     return EditorEventListener::MouseDown(aMouseEvent);
   }
 
-  // XXX This method may change selection. So, we need to commit composition
-  //     here, first.
-
   // Detect only "context menu" click
   // XXX This should be easier to do!
   // But eDOMEvents_contextmenu and eContextMenu is not exposed in any event
   // interface :-(
   int16_t buttonNumber;
   nsresult rv = aMouseEvent->GetButton(&buttonNumber);
   NS_ENSURE_SUCCESS(rv, rv);