From b9c51d6caaaed67def71db275d7f2e58e27267a2 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Fri, 5 Jun 2015 02:06:09 +0900 Subject: [PATCH] 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 | 15 +++++++++++++ dom/events/TextComposition.h | 6 ++++++ editor/libeditor/nsEditor.cpp | 29 +++++++++++++++++++++----- editor/libeditor/nsEditor.h | 4 ++++ editor/libeditor/nsPlaintextEditor.cpp | 2 +- 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/dom/events/TextComposition.cpp b/dom/events/TextComposition.cpp index 68f01084602..72573e83121 100644 --- a/dom/events/TextComposition.cpp +++ b/dom/events/TextComposition.cpp @@ -462,6 +462,21 @@ TextComposition::EditorWillHandleCompositionChangeEvent( "attribute value of the latest compositionupdate event"); } +void +TextComposition::OnEditorDestroyed() +{ + MOZ_ASSERT(!mIsEditorHandlingEvent, + "The editor should have stopped listening events"); + nsCOMPtr 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() { diff --git a/dom/events/TextComposition.h b/dom/events/TextComposition.h index ee50afc3edb..4b182b3bbdf 100644 --- a/dom/events/TextComposition.h +++ b/dom/events/TextComposition.h @@ -124,6 +124,12 @@ public: 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 diff --git a/editor/libeditor/nsEditor.cpp b/editor/libeditor/nsEditor.cpp index 0a6d33b7af6..51398066c4b 100644 --- a/editor/libeditor/nsEditor.cpp +++ b/editor/libeditor/nsEditor.cpp @@ -152,6 +152,10 @@ nsEditor::~nsEditor() { NS_ASSERTION(!mDocWeak || mDidPreDestroy, "Why PreDestroy hasn't been called?"); + if (mComposition) { + mComposition->OnEditorDestroyed(); + mComposition = nullptr; + } mTxnMgr = nullptr; delete mPhonetic; @@ -345,7 +349,12 @@ nsEditor::InstallEventListeners() nsEditorEventListener* listener = reinterpret_cast(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 @@ -356,8 +365,9 @@ nsEditor::RemoveEventListeners() } reinterpret_cast(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; } @@ -2268,7 +2278,7 @@ nsEditor::InsertTextImpl(const nsAString& aStringToInsert, NS_ENSURE_TRUE(aInOutNode && *aInOutNode && aInOutOffset && aDoc, NS_ERROR_NULL_POINTER); - if (!mComposition && aStringToInsert.IsEmpty()) { + if (!ShouldHandleIMEComposition() && aStringToInsert.IsEmpty()) { return NS_OK; } @@ -2307,7 +2317,7 @@ nsEditor::InsertTextImpl(const nsAString& aStringToInsert, } nsresult res; - if (mComposition) { + if (ShouldHandleIMEComposition()) { if (!node->IsNodeOfType(nsINode::eTEXT)) { // create a text node nsRefPtr newNode = aDoc->CreateTextNode(EmptyString()); @@ -2356,7 +2366,7 @@ nsEditor::InsertTextIntoTextNodeImpl(const nsAString& aStringToInsert, // 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; @@ -4023,6 +4033,15 @@ 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() { diff --git a/editor/libeditor/nsEditor.h b/editor/libeditor/nsEditor.h index 50ff94b2472..ff2e0573f96 100644 --- a/editor/libeditor/nsEditor.h +++ b/editor/libeditor/nsEditor.h @@ -589,6 +589,10 @@ public: * 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); diff --git a/editor/libeditor/nsPlaintextEditor.cpp b/editor/libeditor/nsPlaintextEditor.cpp index 75028aac8b8..0af8de56009 100644 --- a/editor/libeditor/nsPlaintextEditor.cpp +++ b/editor/libeditor/nsPlaintextEditor.cpp @@ -710,7 +710,7 @@ NS_IMETHODIMP nsPlaintextEditor::InsertText(const nsAString &aStringToInsert) nsCOMPtr kungFuDeathGrip(mRules); EditAction opID = EditAction::insertText; - if (mComposition) { + if (ShouldHandleIMEComposition()) { opID = EditAction::insertIMEText; } nsAutoPlaceHolderBatch batch(this, nullptr);