Bug 643786. Don't fire DOMNodeRemoved when removing the editor created <br>. r=ehsan

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<nsIRunnable>* 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);                   \

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<nsIRunnable>* 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<nsIContent*>(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;
   }

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

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<nsIHTMLEditor> kungFuDeathGrip(mHTMLEditor);
   nsCOMPtr<nsISelection> selection;
   nsresult res = mHTMLEditor->GetSelection(getter_AddRefs(selection));