Bug 1162818 part.1 nsEditor shouldn't release/forget mComposition becuase it should be handled by it after reframing r=ehsan
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 05 Jun 2015 02:06:09 +0900
changeset 247169 66bab785635f3b108bc88c07228848463a07d7f6
parent 247168 b4aac564d1e066866be2bd83a6aad9d8778d9cbf
child 247170 e037403d3e8c1e82956046c0b208864079a96b0b
push id28855
push userkwierso@gmail.com
push dateFri, 05 Jun 2015 01:19:30 +0000
treeherdermozilla-central@227d356ac030 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1162818
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1162818 part.1 nsEditor shouldn't release/forget mComposition becuase it should be handled by it after reframing r=ehsan
dom/events/TextComposition.cpp
dom/events/TextComposition.h
editor/libeditor/nsEditor.cpp
editor/libeditor/nsEditor.h
editor/libeditor/nsPlaintextEditor.cpp
--- a/dom/events/TextComposition.cpp
+++ b/dom/events/TextComposition.cpp
@@ -458,16 +458,31 @@ TextComposition::EditorWillHandleComposi
   mIsEditorHandlingEvent = true;
 
   MOZ_ASSERT(mLastData == aCompositionChangeEvent->mData,
     "The text of a compositionchange event must be same as previous data "
     "attribute value of the latest compositionupdate event");
 }
 
 void
+TextComposition::OnEditorDestroyed()
+{
+  MOZ_ASSERT(!mIsEditorHandlingEvent,
+             "The editor should have stopped listening events");
+  nsCOMPtr<nsIWidget> widget = GetWidget();
+  if (NS_WARN_IF(!widget)) {
+    // XXX If this could happen, how do we notify IME of destroying the editor?
+    return;
+  }
+
+  // Try to cancel the composition.
+  RequestToCommit(widget, true);
+}
+
+void
 TextComposition::EditorDidHandleCompositionChangeEvent()
 {
   mString = mLastData;
   mIsEditorHandlingEvent = false;
 }
 
 void
 TextComposition::StartHandlingComposition(nsIEditor* aEditor)
--- a/dom/events/TextComposition.h
+++ b/dom/events/TextComposition.h
@@ -120,16 +120,22 @@ public:
   /**
    * StartHandlingComposition() and EndHandlingComposition() are called by
    * editor when it holds a TextComposition instance and release it.
    */
   void StartHandlingComposition(nsIEditor* aEditor);
   void EndHandlingComposition(nsIEditor* aEditor);
 
   /**
+   * OnEditorDestroyed() is called when the editor is destroyed but there is
+   * active composition.
+   */
+  void OnEditorDestroyed();
+
+  /**
    * CompositionChangeEventHandlingMarker class should be created at starting
    * to handle text event in focused editor.  This calls
    * EditorWillHandleCompositionChangeEvent() and
    * EditorDidHandleCompositionChangeEvent() automatically.
    */
   class MOZ_STACK_CLASS CompositionChangeEventHandlingMarker
   {
   public:
--- a/editor/libeditor/nsEditor.cpp
+++ b/editor/libeditor/nsEditor.cpp
@@ -147,16 +147,20 @@ nsEditor::nsEditor()
 ,  mIsInEditAction(false)
 {
 }
 
 nsEditor::~nsEditor()
 {
   NS_ASSERTION(!mDocWeak || mDidPreDestroy, "Why PreDestroy hasn't been called?");
 
+  if (mComposition) {
+    mComposition->OnEditorDestroyed();
+    mComposition = nullptr;
+  }
   mTxnMgr = nullptr;
 
   delete mPhonetic;
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsEditor)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsEditor)
@@ -340,29 +344,35 @@ nsEditor::InstallEventListeners()
   // Initialize the event target.
   nsCOMPtr<nsIContent> rootContent = GetRoot();
   NS_ENSURE_TRUE(rootContent, NS_ERROR_NOT_AVAILABLE);
   mEventTarget = do_QueryInterface(rootContent->GetParent());
   NS_ENSURE_TRUE(mEventTarget, NS_ERROR_NOT_AVAILABLE);
 
   nsEditorEventListener* listener =
     reinterpret_cast<nsEditorEventListener*>(mEventListener.get());
-  return listener->Connect(this);
+  nsresult rv = listener->Connect(this);
+  if (mComposition) {
+    // Restart to handle composition with new editor contents.
+    mComposition->StartHandlingComposition(this);
+  }
+  return rv;
 }
 
 void
 nsEditor::RemoveEventListeners()
 {
   if (!mDocWeak || !mEventListener) {
     return;
   }
   reinterpret_cast<nsEditorEventListener*>(mEventListener.get())->Disconnect();
   if (mComposition) {
+    // Even if this is called, don't release mComposition because this is
+    // may be reused after reframing.
     mComposition->EndHandlingComposition(this);
-    mComposition = nullptr;
   }
   mEventTarget = nullptr;
 }
 
 bool
 nsEditor::GetDesiredSpellCheckState()
 {
   // Check user override on this element
@@ -2263,17 +2273,17 @@ nsEditor::InsertTextImpl(const nsAString
                          nsIDocument* aDoc)
 {
   // NOTE: caller *must* have already used nsAutoTxnsConserveSelection
   // stack-based class to turn off txn selection updating.  Caller also turned
   // on rules sniffing if desired.
 
   NS_ENSURE_TRUE(aInOutNode && *aInOutNode && aInOutOffset && aDoc,
                  NS_ERROR_NULL_POINTER);
-  if (!mComposition && aStringToInsert.IsEmpty()) {
+  if (!ShouldHandleIMEComposition() && aStringToInsert.IsEmpty()) {
     return NS_OK;
   }
 
   nsCOMPtr<nsINode> node = *aInOutNode;
   uint32_t offset = static_cast<uint32_t>(*aInOutOffset);
 
   if (!node->IsNodeOfType(nsINode::eTEXT) && IsPlaintextEditor()) {
     nsCOMPtr<nsINode> root = GetRoot();
@@ -2302,17 +2312,17 @@ nsEditor::InsertTextImpl(const nsAString
         offset = node->Length();
       } else if (node->GetParentNode() && node->GetParentNode() == root) {
         node = node->GetParentNode();
       }
     }
   }
 
   nsresult res;
-  if (mComposition) {
+  if (ShouldHandleIMEComposition()) {
     if (!node->IsNodeOfType(nsINode::eTEXT)) {
       // create a text node
       nsRefPtr<nsTextNode> newNode = aDoc->CreateTextNode(EmptyString());
       // then we insert it into the dom tree
       res = InsertNode(*newNode, *node, offset);
       NS_ENSURE_SUCCESS(res, res);
       node = newNode;
       offset = 0;
@@ -2351,17 +2361,17 @@ nsEditor::InsertTextIntoTextNodeImpl(con
                                      Text& aTextNode,
                                      int32_t aOffset, bool aSuppressIME)
 {
   nsRefPtr<EditTxn> txn;
   bool isIMETransaction = false;
   // aSuppressIME is used when editor must insert text, yet this text is not
   // part of the current IME operation. Example: adjusting whitespace around an
   // IME insertion.
-  if (mComposition && !aSuppressIME) {
+  if (ShouldHandleIMEComposition() && !aSuppressIME) {
     if (!mIMETextNode) {
       mIMETextNode = &aTextNode;
       mIMETextOffset = aOffset;
     }
     // Modify mPhonetic with raw text input clauses.
     const TextRangeArray* ranges = mComposition->GetRanges();
     for (uint32_t i = 0; i < (ranges ? ranges->Length() : 0); ++i) {
       const TextRange& textRange = ranges->ElementAt(i);
@@ -4018,16 +4028,25 @@ nsEditor::GetComposition() const
 }
 
 bool
 nsEditor::IsIMEComposing() const
 {
   return mComposition && mComposition->IsComposing();
 }
 
+bool
+nsEditor::ShouldHandleIMEComposition() const
+{
+  // When the editor is being reframed, the old value may be restored with
+  // InsertText().  In this time, the text should be inserted as not a part
+  // of the composition.
+  return mComposition && mDidPostCreate;
+}
+
 nsresult
 nsEditor::DeleteSelectionAndPrepareToCreateNode()
 {
   nsresult res;
   nsRefPtr<Selection> selection = GetSelection();
   NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
   MOZ_ASSERT(selection->GetAnchorFocusRange());
 
--- a/editor/libeditor/nsEditor.h
+++ b/editor/libeditor/nsEditor.h
@@ -584,16 +584,20 @@ public:
   /**
    * Returns current composition.
    */
   mozilla::TextComposition* GetComposition() const;
   /**
    * Returns true if there is composition string and not fixed.
    */
   bool IsIMEComposing() const;
+  /**
+   * Returns true when inserting text should be a part of current composition.
+   */
+  bool ShouldHandleIMEComposition() const;
 
   /** from html rules code - migration in progress */
   static nsresult GetTagString(nsIDOMNode *aNode, nsAString& outString);
   static nsIAtom *GetTag(nsIDOMNode *aNode);
 
   bool NodesSameType(nsIDOMNode *aNode1, nsIDOMNode *aNode2);
   virtual bool AreNodesSameType(nsIContent* aNode1, nsIContent* aNode2);
 
--- a/editor/libeditor/nsPlaintextEditor.cpp
+++ b/editor/libeditor/nsPlaintextEditor.cpp
@@ -705,17 +705,17 @@ nsPlaintextEditor::DeleteSelection(EDire
 NS_IMETHODIMP nsPlaintextEditor::InsertText(const nsAString &aStringToInsert)
 {
   if (!mRules) { return NS_ERROR_NOT_INITIALIZED; }
 
   // Protect the edit rules object from dying
   nsCOMPtr<nsIEditRules> kungFuDeathGrip(mRules);
 
   EditAction opID = EditAction::insertText;
-  if (mComposition) {
+  if (ShouldHandleIMEComposition()) {
     opID = EditAction::insertIMEText;
   }
   nsAutoPlaceHolderBatch batch(this, nullptr);
   nsAutoRules beginRulesSniffing(this, opID, nsIEditor::eNext);
 
   // pre-process
   nsRefPtr<Selection> selection = GetSelection();
   NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);