From 2160f5e6fc533005f646f21800de5c69569b2aa0 Mon Sep 17 00:00:00 2001 From: Jonas Sicking Date: Mon, 9 May 2011 12:33:04 -0700 Subject: [PATCH] Bug 643786. Don't fire DOMNodeRemoved when removing the editor created
. r=ehsan --- content/base/public/nsContentUtils.h | 20 +++++++++++++++++++ content/base/src/nsContentUtils.cpp | 20 +++++++++++++++++-- .../libeditor/html/crashtests/crashtests.list | 1 + editor/libeditor/html/nsHTMLEditRules.cpp | 3 +++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h index 2d22462431c0..47f16e338db8 100644 --- a/content/base/public/nsContentUtils.h +++ b/content/base/public/nsContentUtils.h @@ -130,6 +130,7 @@ class nsIMIMEHeaderParam; class nsIObserver; class nsPresContext; class nsIChannel; +class nsAutoScriptBlockerSuppressNodeRemoved; struct nsIntMargin; class nsPIDOMWindow; class nsIDocumentLoaderFactory; @@ -184,6 +185,7 @@ struct nsShortcutCandidate { class nsContentUtils { + friend class nsAutoScriptBlockerSuppressNodeRemoved; typedef mozilla::dom::Element Element; public: @@ -1913,6 +1915,9 @@ private: static PRBool sInitialized; static PRUint32 sScriptBlockerCount; static PRUint32 sRemovableScriptBlockerCount; +#ifdef DEBUG + static PRUint32 sDOMNodeRemovedSuppressCount; +#endif static nsCOMArray* sBlockedScriptRunners; static PRUint32 sRunnersCountAtFirstBlocker; static PRUint32 sScriptBlockerCountWhereRunnersPrevented; @@ -2004,6 +2009,21 @@ private: MOZILLA_DECL_USE_GUARD_OBJECT_NOTIFIER }; +class NS_STACK_CLASS nsAutoScriptBlockerSuppressNodeRemoved : + public nsAutoScriptBlocker { +public: + nsAutoScriptBlockerSuppressNodeRemoved() { +#ifdef DEBUG + ++nsContentUtils::sDOMNodeRemovedSuppressCount; +#endif + } + ~nsAutoScriptBlockerSuppressNodeRemoved() { +#ifdef DEBUG + --nsContentUtils::sDOMNodeRemovedSuppressCount; +#endif + } +}; + #define NS_INTERFACE_MAP_ENTRY_TEAROFF(_interface, _allocator) \ if (aIID.Equals(NS_GET_IID(_interface))) { \ foundInterface = static_cast<_interface *>(_allocator); \ diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp index 1da849a42444..6c239fb7aa27 100644 --- a/content/base/src/nsContentUtils.cpp +++ b/content/base/src/nsContentUtils.cpp @@ -254,6 +254,9 @@ nsIBidiKeyboard *nsContentUtils::sBidiKeyboard = nsnull; #endif PRUint32 nsContentUtils::sScriptBlockerCount = 0; PRUint32 nsContentUtils::sRemovableScriptBlockerCount = 0; +#ifdef DEBUG +PRUint32 nsContentUtils::sDOMNodeRemovedSuppressCount = 0; +#endif nsCOMArray* nsContentUtils::sBlockedScriptRunners = nsnull; PRUint32 nsContentUtils::sRunnersCountAtFirstBlocker = 0; PRUint32 nsContentUtils::sScriptBlockerCountWhereRunnersPrevented = 0; @@ -3729,15 +3732,28 @@ nsContentUtils::MaybeFireNodeRemoved(nsINode* aChild, nsINode* aParent, NS_PRECONDITION(aChild->GetNodeParent() == aParent, "Wrong parent"); NS_PRECONDITION(aChild->GetOwnerDoc() == aOwnerDoc, "Wrong owner-doc"); + // This checks that IsSafeToRunScript is true since we don't want to fire + // events when that is false. We can't rely on nsEventDispatcher to assert + // this in this situation since most of the time there are no mutation + // event listeners, in which case we won't even attempt to dispatch events. + // However this also allows for two exceptions. First off, we don't assert + // if the mutation happens to native anonymous content since we never fire + // mutation events on such content anyway. + // Second, we don't assert if sDOMNodeRemovedSuppressCount is true since + // that is a know case when we'd normally fire a mutation event, but can't + // make that safe and so we suppress it at this time. Ideally this should + // go away eventually. NS_ASSERTION(aChild->IsNodeOfType(nsINode::eCONTENT) && static_cast(aChild)-> IsInNativeAnonymousSubtree() || - IsSafeToRunScript(), + IsSafeToRunScript() || + sDOMNodeRemovedSuppressCount, "Want to fire DOMNodeRemoved event, but it's not safe"); // Having an explicit check here since it's an easy mistake to fall into, // and there might be existing code with problems. We'd rather be safe - // than fire DOMNodeRemoved in all corner cases. + // than fire DOMNodeRemoved in all corner cases. We also rely on it for + // nsAutoScriptBlockerSuppressNodeRemoved. if (!IsSafeToRunScript()) { return; } diff --git a/editor/libeditor/html/crashtests/crashtests.list b/editor/libeditor/html/crashtests/crashtests.list index 9b5395ddb7e3..d8a326fb3934 100644 --- a/editor/libeditor/html/crashtests/crashtests.list +++ b/editor/libeditor/html/crashtests/crashtests.list @@ -21,3 +21,4 @@ load 582138-1.xhtml load 612565-1.html asserts(6) load 615015-1.html # Bug 439258 load 615450-1.html +load 643786-1.html diff --git a/editor/libeditor/html/nsHTMLEditRules.cpp b/editor/libeditor/html/nsHTMLEditRules.cpp index 4cd7355c8c25..eb5fd2b069e2 100644 --- a/editor/libeditor/html/nsHTMLEditRules.cpp +++ b/editor/libeditor/html/nsHTMLEditRules.cpp @@ -9202,6 +9202,9 @@ nsHTMLEditRules::DocumentModifiedWorker() return; } + // DeleteNode below may cause a flush, which could destroy the editor + nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker; + nsCOMPtr kungFuDeathGrip(mHTMLEditor); nsCOMPtr selection; nsresult res = mHTMLEditor->GetSelection(getter_AddRefs(selection));