Bug 496360 part.2 IMEContentObserver shouldn't dispatch TextChangeEvent while editor handles an edit action r=ehsan+smaug, sr=smaug

This commit is contained in:
Masayuki Nakano 2014-07-31 13:37:59 +09:00
parent d67dabd004
commit 5805e688a3
7 changed files with 230 additions and 56 deletions

View File

@ -930,6 +930,18 @@ nsTextInputListener::EditAction()
return NS_OK;
}
NS_IMETHODIMP
nsTextInputListener::BeforeEditAction()
{
return NS_OK;
}
NS_IMETHODIMP
nsTextInputListener::CancelEditAction()
{
return NS_OK;
}
// END nsIEditorObserver

View File

@ -25,6 +25,7 @@
#include "nsISelectionController.h"
#include "nsISelectionPrivate.h"
#include "nsISupports.h"
#include "nsITextControlElement.h"
#include "nsIWidget.h"
#include "nsPresContext.h"
#include "nsThreadUtils.h"
@ -34,9 +35,32 @@ namespace mozilla {
using namespace widget;
NS_IMPL_CYCLE_COLLECTION(IMEContentObserver,
mWidget, mSelection,
mRootContent, mEditableNode, mDocShell)
NS_IMPL_CYCLE_COLLECTION_CLASS(IMEContentObserver)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(IMEContentObserver)
nsAutoScriptBlocker scriptBlocker;
tmp->UnregisterObservers(true);
NS_IMPL_CYCLE_COLLECTION_UNLINK(mWidget)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mSelection)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mRootContent)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mEditableNode)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mEditor)
tmp->mUpdatePreference.mWantUpdates = nsIMEUpdatePreference::NOTIFY_NOTHING;
tmp->mESM = nullptr;
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(IMEContentObserver)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mWidget)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSelection)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRootContent)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEditableNode)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocShell)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEditor)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IMEContentObserver)
NS_INTERFACE_MAP_ENTRY(nsISelectionListener)
@ -44,6 +68,7 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IMEContentObserver)
NS_INTERFACE_MAP_ENTRY(nsIReflowObserver)
NS_INTERFACE_MAP_ENTRY(nsIScrollObserver)
NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
NS_INTERFACE_MAP_ENTRY(nsIEditorObserver)
NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsISelectionListener)
NS_INTERFACE_MAP_END
@ -52,6 +77,7 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(IMEContentObserver)
IMEContentObserver::IMEContentObserver()
: mESM(nullptr)
, mIsEditorInTransaction(false)
{
#ifdef DEBUG
TestMergingTextChangeData();
@ -72,6 +98,28 @@ IMEContentObserver::Init(nsIWidget* aWidget,
return;
}
nsCOMPtr<nsITextControlElement> textControlElement =
do_QueryInterface(mEditableNode);
if (textControlElement) {
// This may fail. For example, <input type="button" contenteditable>
mEditor = textControlElement->GetTextEditor();
if (!mEditor && mEditableNode->IsContent()) {
// The element must be an editing host.
nsIContent* editingHost = mEditableNode->AsContent()->GetEditingHost();
MOZ_ASSERT(editingHost == mEditableNode,
"found editing host should be mEditableNode");
if (editingHost == mEditableNode) {
mEditor = nsContentUtils::GetHTMLEditor(aPresContext);
}
}
} else {
mEditor = nsContentUtils::GetHTMLEditor(aPresContext);
}
MOZ_ASSERT(mEditor, "Failed to get editor");
if (mEditor) {
mEditor->AddEditorObserver(this);
}
nsIPresShell* presShell = aPresContext->PresShell();
// get selection and root content
@ -159,34 +207,62 @@ IMEContentObserver::ObserveEditableNode()
}
void
IMEContentObserver::Destroy()
IMEContentObserver::UnregisterObservers(bool aPostEvent)
{
// If CreateTextStateManager failed, mRootContent will be null,
// and we should not call NotifyIME(IMENotification(NOTIFY_IME_OF_BLUR))
if (mRootContent) {
if (mEditor) {
mEditor->RemoveEditorObserver(this);
}
// If CreateTextStateManager failed, mRootContent will be null, then, we
// should not call NotifyIME(IMENotification(NOTIFY_IME_OF_BLUR))
if (mRootContent && mWidget) {
if (IMEStateManager::IsTestingIME() && mEditableNode) {
nsIDocument* doc = mEditableNode->OwnerDoc();
(new AsyncEventDispatcher(doc, NS_LITERAL_STRING("MozIMEFocusOut"),
false, false))->RunDOMEventWhenSafe();
if (doc) {
nsRefPtr<AsyncEventDispatcher> dispatcher =
new AsyncEventDispatcher(doc, NS_LITERAL_STRING("MozIMEFocusOut"),
false, false);
if (aPostEvent) {
dispatcher->PostDOMEvent();
} else {
dispatcher->RunDOMEventWhenSafe();
}
}
}
// A test event handler might destroy the widget.
if (mWidget) {
mWidget->NotifyIME(IMENotification(NOTIFY_IME_OF_BLUR));
}
mWidget->NotifyIME(IMENotification(NOTIFY_IME_OF_BLUR));
}
// Even if there are some pending notification, it'll never notify the widget.
mWidget = nullptr;
if (mUpdatePreference.WantSelectionChange() && mSelection) {
nsCOMPtr<nsISelectionPrivate> selPrivate(do_QueryInterface(mSelection));
if (selPrivate) {
selPrivate->RemoveSelectionListener(this);
}
}
mSelection = nullptr;
if (mUpdatePreference.WantTextChange() && mRootContent) {
mRootContent->RemoveMutationObserver(this);
}
if (mUpdatePreference.WantPositionChanged() && mDocShell) {
mDocShell->RemoveWeakScrollObserver(this);
mDocShell->RemoveWeakReflowObserver(this);
}
}
void
IMEContentObserver::Destroy()
{
// WARNING: When you change this method, you have to check Unlink() too.
UnregisterObservers(false);
mEditor = nullptr;
// Even if there are some pending notification, it'll never notify the widget.
mWidget = nullptr;
mSelection = nullptr;
mRootContent = nullptr;
mEditableNode = nullptr;
mDocShell = nullptr;
@ -378,7 +454,7 @@ private:
IMEContentObserver::TextChangeData mData;
};
bool
void
IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
{
MOZ_ASSERT(aTextChangeData.mStartOffset <= aTextChangeData.mRemovedEndOffset,
@ -389,7 +465,7 @@ IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
if (!mTextChangeData.mStored) {
mTextChangeData = aTextChangeData;
MOZ_ASSERT(mTextChangeData.mStored, "Why mStored is false?");
return true;
return;
}
// |mTextChangeData| should represent all modified text ranges and all
@ -446,7 +522,7 @@ IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
std::max(newRemovedEndOffsetInOldText, oldData.mRemovedEndOffset);
// The new end offset of added text is always the larger offset.
mTextChangeData.mAddedEndOffset = newData.mAddedEndOffset;
return false;
return;
}
if (newData.mStartOffset >= oldData.mStartOffset) {
@ -473,7 +549,7 @@ IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
// always same or larger. Therefore, the merged end offset of added
// text should be the new end offset of added text.
mTextChangeData.mAddedEndOffset = newData.mAddedEndOffset;
return false;
return;
}
// Case 3:
@ -491,7 +567,7 @@ IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
oldData.mAddedEndOffset + newData.Difference();
mTextChangeData.mAddedEndOffset =
std::max(newData.mAddedEndOffset, oldAddedEndOffsetInNewText);
return false;
return;
}
if (newData.mRemovedEndOffset >= oldData.mStartOffset) {
@ -522,7 +598,7 @@ IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
// it. Therefore, merged end offset of added text is always the new end
// offset of added text.
mTextChangeData.mAddedEndOffset = newData.mAddedEndOffset;
return false;
return;
}
// Case 5:
@ -542,7 +618,7 @@ IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
oldData.mAddedEndOffset + newData.Difference();
mTextChangeData.mAddedEndOffset =
std::max(newData.mAddedEndOffset, oldAddedEndOffsetInNewText);
return false;
return;
}
// Case 6:
@ -561,8 +637,6 @@ IMEContentObserver::StoreTextChangeData(const TextChangeData& aTextChangeData)
oldData.mAddedEndOffset + newData.Difference();
mTextChangeData.mAddedEndOffset =
std::max(newData.mAddedEndOffset, oldAddedEndOffsetInNewText);
return false;
}
void
@ -592,9 +666,8 @@ IMEContentObserver::CharacterDataChanged(nsIDocument* aDocument,
uint32_t newEnd = offset + aInfo->mReplaceLength;
TextChangeData data(offset, oldEnd, newEnd, causedByComposition);
if (StoreTextChangeData(data)) {
nsContentUtils::AddScriptRunner(new TextChangeEvent(this, mTextChangeData));
}
StoreTextChangeData(data);
FlushMergeableNotifications();
}
void
@ -629,9 +702,8 @@ IMEContentObserver::NotifyContentAdded(nsINode* aContainer,
TextChangeData data(offset, offset, offset + addingLength,
causedByComposition);
if (StoreTextChangeData(data)) {
nsContentUtils::AddScriptRunner(new TextChangeEvent(this, mTextChangeData));
}
StoreTextChangeData(data);
FlushMergeableNotifications();
}
void
@ -693,9 +765,8 @@ IMEContentObserver::ContentRemoved(nsIDocument* aDocument,
}
TextChangeData data(offset, offset + textLength, offset, causedByComposition);
if (StoreTextChangeData(data)) {
nsContentUtils::AddScriptRunner(new TextChangeEvent(this, mTextChangeData));
}
StoreTextChangeData(data);
FlushMergeableNotifications();
}
static nsIContent*
@ -752,8 +823,43 @@ IMEContentObserver::AttributeChanged(nsIDocument* aDocument,
TextChangeData data(start, start + mPreAttrChangeLength,
start + postAttrChangeLength, causedByComposition);
if (StoreTextChangeData(data)) {
StoreTextChangeData(data);
FlushMergeableNotifications();
}
NS_IMETHODIMP
IMEContentObserver::EditAction()
{
mIsEditorInTransaction = false;
FlushMergeableNotifications();
return NS_OK;
}
NS_IMETHODIMP
IMEContentObserver::BeforeEditAction()
{
mIsEditorInTransaction = true;
return NS_OK;
}
NS_IMETHODIMP
IMEContentObserver::CancelEditAction()
{
mIsEditorInTransaction = false;
FlushMergeableNotifications();
return NS_OK;
}
void
IMEContentObserver::FlushMergeableNotifications()
{
if (mIsEditorInTransaction) {
return;
}
if (mTextChangeData.mStored) {
nsContentUtils::AddScriptRunner(new TextChangeEvent(this, mTextChangeData));
mTextChangeData.mStored = false;
}
}

View File

@ -11,6 +11,8 @@
#include "nsCOMPtr.h"
#include "nsCycleCollectionParticipant.h"
#include "nsIDocShell.h" // XXX Why does only this need to be included here?
#include "nsIEditor.h"
#include "nsIEditorObserver.h"
#include "nsIReflowObserver.h"
#include "nsISelectionListener.h"
#include "nsIScrollObserver.h"
@ -29,11 +31,12 @@ class EventStateManager;
// IMEContentObserver notifies widget of any text and selection changes
// in the currently focused editor
class IMEContentObserver MOZ_FINAL : public nsISelectionListener,
public nsStubMutationObserver,
public nsIReflowObserver,
public nsIScrollObserver,
public nsSupportsWeakReference
class IMEContentObserver MOZ_FINAL : public nsISelectionListener
, public nsStubMutationObserver
, public nsIReflowObserver
, public nsIScrollObserver
, public nsSupportsWeakReference
, public nsIEditorObserver
{
public:
IMEContentObserver();
@ -41,6 +44,7 @@ public:
NS_DECL_CYCLE_COLLECTING_ISUPPORTS
NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(IMEContentObserver,
nsISelectionListener)
NS_DECL_NSIEDITOROBSERVER
NS_DECL_NSISELECTIONLISTENER
NS_DECL_NSIMUTATIONOBSERVER_CHARACTERDATACHANGED
NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
@ -124,8 +128,14 @@ private:
void NotifyContentAdded(nsINode* aContainer, int32_t aStart, int32_t aEnd);
void ObserveEditableNode();
// Returns true if there is no pending data.
bool StoreTextChangeData(const TextChangeData& aTextChangeData);
/**
* UnregisterObservers() unresiters all listeners and observers.
* @param aPostEvent When true, DOM event will be posted to the thread.
* Otherwise, dispatched when safe.
*/
void UnregisterObservers(bool aPostEvent);
void StoreTextChangeData(const TextChangeData& aTextChangeData);
void FlushMergeableNotifications();
#ifdef DEBUG
void TestMergingTextChangeData();
@ -136,6 +146,7 @@ private:
nsCOMPtr<nsIContent> mRootContent;
nsCOMPtr<nsINode> mEditableNode;
nsCOMPtr<nsIDocShell> mDocShell;
nsCOMPtr<nsIEditor> mEditor;
TextChangeData mTextChangeData;
@ -143,6 +154,8 @@ private:
nsIMEUpdatePreference mUpdatePreference;
uint32_t mPreAttrChangeLength;
bool mIsEditorInTransaction;
};
} // namespace mozilla

View File

@ -896,6 +896,7 @@ nsEditor::BeginPlaceHolderTransaction(nsIAtom *aName)
NS_PRECONDITION(mPlaceHolderBatch >= 0, "negative placeholder batch count!");
if (!mPlaceHolderBatch)
{
NotifyEditorObservers(eNotifyEditorObserversOfBefore);
// time to turn on the batch
BeginUpdateViewBatch();
mPlaceHolderTxn = nullptr;
@ -978,8 +979,10 @@ nsEditor::EndPlaceHolderTransaction()
// notify editor observers of action but if composing, it's done by
// text event handler.
if (!mComposition) {
NotifyEditorObservers();
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
}
} else {
NotifyEditorObservers(eNotifyEditorObserversOfCancel);
}
}
mPlaceHolderBatch--;
@ -1854,17 +1857,35 @@ private:
bool mIsComposing;
};
void nsEditor::NotifyEditorObservers(void)
void
nsEditor::NotifyEditorObservers(NotificationForEditorObservers aNotification)
{
for (int32_t i = 0; i < mEditorObservers.Count(); i++) {
mEditorObservers[i]->EditAction();
}
switch (aNotification) {
case eNotifyEditorObserversOfEnd:
for (int32_t i = 0; i < mEditorObservers.Count(); i++) {
mEditorObservers[i]->EditAction();
}
if (!mDispatchInputEvent) {
return;
}
if (!mDispatchInputEvent) {
return;
}
FireInputEvent();
FireInputEvent();
break;
case eNotifyEditorObserversOfBefore:
for (int32_t i = 0; i < mEditorObservers.Count(); i++) {
mEditorObservers[i]->BeforeEditAction();
}
break;
case eNotifyEditorObserversOfCancel:
for (int32_t i = 0; i < mEditorObservers.Count(); i++) {
mEditorObservers[i]->CancelEditAction();
}
break;
default:
MOZ_CRASH("Handle all notifications here");
break;
}
}
void
@ -2076,7 +2097,7 @@ nsEditor::EndIMEComposition()
mComposition = nullptr;
// notify editor observers of action
NotifyEditorObservers();
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
}

View File

@ -178,7 +178,13 @@ public:
already_AddRefed<nsIDocument> GetDocument();
already_AddRefed<nsIPresShell> GetPresShell();
already_AddRefed<nsIWidget> GetWidget();
void NotifyEditorObservers();
enum NotificationForEditorObservers
{
eNotifyEditorObserversOfEnd,
eNotifyEditorObserversOfBefore,
eNotifyEditorObserversOfCancel
};
void NotifyEditorObservers(NotificationForEditorObservers aNotification);
/* ------------ nsIEditor methods -------------- */
NS_DECL_NSIEDITOR

View File

@ -865,6 +865,8 @@ nsPlaintextEditor::UpdateIMEComposition(nsIDOMEvent* aDOMTextEvent)
nsresult rv = GetSelection(getter_AddRefs(selection));
NS_ENSURE_SUCCESS(rv, rv);
NotifyEditorObservers(eNotifyEditorObserversOfBefore);
nsRefPtr<nsCaret> caretP = ps->GetCaret();
{
@ -885,7 +887,7 @@ nsPlaintextEditor::UpdateIMEComposition(nsIDOMEvent* aDOMTextEvent)
// notified at followed compositionend event.
// NOTE: We must notify after the auto batch will be gone.
if (IsIMEComposing()) {
NotifyEditorObservers();
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
}
return rv;
@ -1100,6 +1102,8 @@ nsPlaintextEditor::Undo(uint32_t aCount)
ForceCompositionEnd();
NotifyEditorObservers(eNotifyEditorObserversOfBefore);
nsAutoRules beginRulesSniffing(this, EditAction::undo, nsIEditor::eNone);
nsTextRulesInfo ruleInfo(EditAction::undo);
@ -1112,8 +1116,8 @@ nsPlaintextEditor::Undo(uint32_t aCount)
result = nsEditor::Undo(aCount);
result = mRules->DidDoAction(selection, &ruleInfo, result);
}
NotifyEditorObservers();
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
return result;
}
@ -1127,6 +1131,8 @@ nsPlaintextEditor::Redo(uint32_t aCount)
ForceCompositionEnd();
NotifyEditorObservers(eNotifyEditorObserversOfBefore);
nsAutoRules beginRulesSniffing(this, EditAction::redo, nsIEditor::eNone);
nsTextRulesInfo ruleInfo(EditAction::redo);
@ -1139,8 +1145,8 @@ nsPlaintextEditor::Redo(uint32_t aCount)
result = nsEditor::Redo(aCount);
result = mRules->DidDoAction(selection, &ruleInfo, result);
}
NotifyEditorObservers();
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
return result;
}

View File

@ -9,7 +9,7 @@
Editor Observer interface to outside world
*/
[scriptable, uuid(e52a09fd-d33a-4f85-be0a-fbd348f0fa27)]
[scriptable, uuid(f3ee57a6-890c-4ce0-a584-8a84bba0292e)]
/**
* A generic editor observer interface.
@ -23,4 +23,14 @@ interface nsIEditorObserver : nsISupports {
* Called after the editor completes a user action.
*/
void EditAction();
/**
* Called when editor starts to handle a user action. I.e., This must be
* called before the first DOM change.
*/
void BeforeEditAction();
/**
* Called after BeforeEditAction() is called but EditorAction() won't be
* called.
*/
void CancelEditAction();
};