Fix for bug 430624 (Crash [@ nsDocShellEditorData::DetachFromWindow] with spellcheck attribute). Patch by cpearce, r=peterv, sr=jst.

content/html/document/src/nsHTMLDocument.cpp

@@ -3971,6 +3971,10 @@ nsHTMLDocument::SetEditingState(EditingState aState)
 nsresult
 nsHTMLDocument::EditingStateChanged()
 {
+  if (mRemovedFromDocShell) {
+    return NS_OK;
+  }
+
   if (mEditingState == eSettingUp || mEditingState == eTearingDown) {
     // XXX We shouldn't recurse.
     return NS_OK;

docshell/base/nsDocShell.cpp

@@ -1011,12 +1011,9 @@ nsDocShell::FirePageHideNotification(PRBool aIsUnload)
                 kids[i]->FirePageHideNotification(aIsUnload);
             }
         }
-    }
-
-    // Now make sure our editor, if any, is detached before we go
-    // any farther.
-    if (mEditorData && aIsUnload) {
-      DetachEditorFromWindow();
+        // Now make sure our editor, if any, is detached before we go
+        // any farther.
+        DetachEditorFromWindow();
     }
 
     return NS_OK;
@@ -3377,11 +3374,6 @@ nsDocShell::Reload(PRUint32 aReloadFlags)
     }
     
 
-    // Need to purge detached editor here, else when we reload a page,
-    // the detached editor state causes SetDesignMode() to fail.
-    if (mOSHE)
-      mOSHE->SetEditorData(nsnull);
-
     return rv;
 }
 
@@ -4885,8 +4877,13 @@ nsDocShell::Embed(nsIContentViewer * aContentViewer,
             SetBaseUrlForWyciwyg(aContentViewer);
     }
     // XXX What if SetupNewViewer fails?
-    if (mLSHE)
+    if (mLSHE) {
+        // Restore the editing state, if it's stored in session history.
+        if (mLSHE->HasDetachedEditor()) {
+            ReattachEditorToWindow(mLSHE);
+        }
         SetHistoryEntry(&mOSHE, mLSHE);
+    }
 
     PRBool updateHistory = PR_TRUE;
 
@@ -5332,29 +5329,20 @@ nsDocShell::CanSavePresentation(PRUint32 aLoadType,
     return PR_TRUE;
 }
 
-PRBool
-nsDocShell::HasDetachedEditor()
-{
-  return (mOSHE && mOSHE->HasDetachedEditor()) ||
-         (mLSHE && mLSHE->HasDetachedEditor());
-}
-
 void
-nsDocShell::ReattachEditorToWindow(nsIDOMWindow *aWindow, nsISHEntry *aSHEntry)
+nsDocShell::ReattachEditorToWindow(nsISHEntry *aSHEntry)
 {
     NS_ASSERTION(!mEditorData,
                  "Why reattach an editor when we already have one?");
-    NS_ASSERTION(aWindow,
-                 "Need a window to reattach to.");
-    NS_ASSERTION(HasDetachedEditor(),
+    NS_ASSERTION(aSHEntry && aSHEntry->HasDetachedEditor(),
                  "Reattaching when there's not a detached editor.");
 
-    if (mEditorData || !aWindow || !aSHEntry)
+    if (mEditorData || !aSHEntry)
       return;
 
     mEditorData = aSHEntry->ForgetEditorData();
     if (mEditorData) {
-        nsresult res = mEditorData->ReattachToWindow(aWindow);
+        nsresult res = mEditorData->ReattachToWindow(this);
         NS_ASSERTION(NS_SUCCEEDED(res), "Failed to reattach editing session");
     }
 }
@@ -5362,18 +5350,21 @@ nsDocShell::ReattachEditorToWindow(nsIDOMWindow *aWindow, nsISHEntry *aSHEntry)
 void
 nsDocShell::DetachEditorFromWindow(nsISHEntry *aSHEntry)
 {
-    if (!aSHEntry || !mEditorData)
+    if (!mEditorData)
         return;
 
-    NS_ASSERTION(!aSHEntry->HasDetachedEditor(),
-                 "Why detach an editor twice?");
+    NS_ASSERTION(!aSHEntry || !aSHEntry->HasDetachedEditor(),
+                 "Detaching editor when it's already detached.");
 
     nsresult res = mEditorData->DetachFromWindow();
     NS_ASSERTION(NS_SUCCEEDED(res), "Failed to detach editor");
 
     if (NS_SUCCEEDED(res)) {
-      // Make aSHEntry hold the owning ref to the editor data.
-      aSHEntry->SetEditorData(mEditorData.forget());
+        // Make aSHEntry hold the owning ref to the editor data.
+        if (aSHEntry)
+            aSHEntry->SetEditorData(mEditorData.forget());
+        else
+            mEditorData = nsnull;
     }
 
 #ifdef DEBUG
@@ -5390,7 +5381,8 @@ nsDocShell::DetachEditorFromWindow(nsISHEntry *aSHEntry)
 void
 nsDocShell::DetachEditorFromWindow()
 {
-    DetachEditorFromWindow(mOSHE);
+    if (mOSHE)
+        DetachEditorFromWindow(mOSHE);
 }
 
 nsresult
@@ -5542,6 +5534,10 @@ nsDocShell::FinishRestore()
         }
     }
 
+    if (mOSHE && mOSHE->HasDetachedEditor()) {
+      ReattachEditorToWindow(mOSHE);
+    }
+
     if (mContentViewer) {
         nsCOMPtr<nsIDOMDocument> domDoc;
         mContentViewer->GetDOMDocument(getter_AddRefs(domDoc));
@@ -6010,11 +6006,6 @@ nsDocShell::RestoreFromHistory()
         }
     }
 
-    if (HasDetachedEditor()) {
-      nsCOMPtr<nsIDOMWindow> domWin = do_QueryInterface(privWin);
-      ReattachEditorToWindow(domWin, mLSHE);
-    }
-
     // Simulate the completion of the load.
     nsDocShell::FinishRestore();
 
@@ -7153,10 +7144,6 @@ nsDocShell::InternalLoad(nsIURI * aURI,
 
     mLoadType = aLoadType;
 
-    // Detach the current editor so that it can be restored from the
-    // bfcache later.
-    DetachEditorFromWindow();
-
     // mLSHE should be assigned to aSHEntry, only after Stop() has
     // been called. But when loading an error page, do not clear the
     // mLSHE for the real page.
@@ -7220,22 +7207,6 @@ nsDocShell::InternalLoad(nsIURI * aURI,
         nsCOMPtr<nsIChannel> chan(do_QueryInterface(req));
         DisplayLoadError(rv, aURI, nsnull, chan);
     }
-    
-    if (aSHEntry) {
-        if (aLoadType & LOAD_CMD_HISTORY) {
-            // We've just loaded a page from session history. Reattach
-            // its editing session if it has one.
-            nsCOMPtr<nsIDOMWindow> domWin;
-            CallGetInterface(this, static_cast<nsIDOMWindow**>(getter_AddRefs(domWin)));
-            ReattachEditorToWindow(domWin, aSHEntry);
-        } else {
-            // This is a non-history load from a session history entry. Purge any
-            // previous editing sessions, so that the the editing session will
-            // be recreated. This can happen when we reload something that's
-            // in the bfcache.
-            aSHEntry->SetEditorData(nsnull);
-        }
-    }
 
     return rv;
 }
@@ -8735,9 +8706,12 @@ nsDocShell::ShouldDiscardLayoutState(nsIHttpChannel * aChannel)
 NS_IMETHODIMP nsDocShell::GetEditor(nsIEditor * *aEditor)
 {
   NS_ENSURE_ARG_POINTER(aEditor);
-  nsresult rv = EnsureEditorData();
-  if (NS_FAILED(rv)) return rv;
-  
+
+  if (!mEditorData) {
+    *aEditor = nsnull;
+    return NS_OK;
+  }
+
   return mEditorData->GetEditor(aEditor);
 }
 
@@ -9043,9 +9017,12 @@ nsDocShell::EnsureScriptEnvironment()
 NS_IMETHODIMP
 nsDocShell::EnsureEditorData()
 {
-    NS_ASSERTION(!HasDetachedEditor(), "EnsureEditorData() called when detached.\n");
-
-    if (!mEditorData && !mIsBeingDestroyed && !HasDetachedEditor()) {
+    PRBool openDocHasDetachedEditor = mOSHE && mOSHE->HasDetachedEditor();
+    if (!mEditorData && !mIsBeingDestroyed && !openDocHasDetachedEditor) {
+        // We shouldn't recreate the editor data if it already exists, or
+        // we're shutting down, or we already have a detached editor data
+        // stored in the session history. We should only have one editordata
+        // per docshell.
         mEditorData = new nsDocShellEditorData(this);
     }
 

docshell/base/nsDocShell.h

@@ -525,7 +525,7 @@ protected:
     // we are it's still OK to load this URI.
     PRBool IsOKToLoadURI(nsIURI* aURI);
     
-    void ReattachEditorToWindow(nsIDOMWindow *aWindow, nsISHEntry *aSHEntry);
+    void ReattachEditorToWindow(nsISHEntry *aSHEntry);
     void DetachEditorFromWindow(nsISHEntry *aSHEntry);
 
 protected:
@@ -677,10 +677,6 @@ protected:
 
     static nsIURIFixup *sURIFixup;
 
-    // Returns true when the currently open document has a detached editor
-    // waiting to be reattached.
-    PRBool HasDetachedEditor();
-
 public:
     class InterfaceRequestorProxy : public nsIInterfaceRequestor {
     public:

docshell/base/nsDocShellEditorData.cpp

@@ -73,12 +73,12 @@ nsDocShellEditorData::~nsDocShellEditorData()
 void
 nsDocShellEditorData::TearDownEditor()
 {
-  NS_ASSERTION(mIsDetached, "We should be detached before tearing down");
   if (mEditor) {
     mEditor->PreDestroy();
     mEditor = nsnull;
   }
   mEditingSession = nsnull;
+  mIsDetached = PR_FALSE;
 }
 
 
@@ -244,12 +244,16 @@ nsDocShellEditorData::DetachFromWindow()
   if (htmlDoc)
     mDetachedEditingState = htmlDoc->GetEditingState();
 
+  mDocShell = nsnull;
+
   return NS_OK;
 }
 
 nsresult
-nsDocShellEditorData::ReattachToWindow(nsIDOMWindow *aWindow)
+nsDocShellEditorData::ReattachToWindow(nsIDocShell* aDocShell)
 {
+  mDocShell = aDocShell;
+
   nsCOMPtr<nsIDOMWindow> domWindow = do_GetInterface(mDocShell);
   nsresult rv = mEditingSession->ReattachToWindow(domWindow);
   NS_ENSURE_SUCCESS(rv, rv);

docshell/base/nsDocShellEditorData.h

@@ -72,7 +72,7 @@ public:
   nsresult SetEditor(nsIEditor *inEditor);
   void TearDownEditor();
   nsresult DetachFromWindow();
-  nsresult ReattachToWindow(nsIDOMWindow *aWindow);
+  nsresult ReattachToWindow(nsIDocShell *aDocShell);
 
 protected:
 

docshell/base/nsIDocShell.idl

@@ -68,7 +68,7 @@ interface nsILayoutHistoryState;
 interface nsISecureBrowserUI;
 interface nsIDOMStorage;
 
-[scriptable, uuid(4b00222a-8d0a-46d7-a1fe-43bd89d19324)]
+[scriptable, uuid(7d1cf6b9-daa3-476d-8f9f-9eb2a971a95c)]
 interface nsIDocShell : nsISupports
 {
   /**

docshell/shistory/public/nsISHEntry.idl

@@ -58,7 +58,7 @@ class nsDocShellEditorData;
 [ptr] native nsDocShellEditorDataPtr(nsDocShellEditorData);
 
 
-[scriptable, uuid(abe54136-49e5-44ca-a749-290038c6b85d)]
+[scriptable, uuid(c16fde76-3108-450e-8c8c-ae8286f286ed)]
 interface nsISHEntry : nsIHistoryEntry
 {
     /** URI for the document */

docshell/test/navigation/Makefile.in

@@ -50,6 +50,7 @@ _TEST_FILES = \
 		test_bug278916.html \
 		test_bug279495.html \
 		test_bug386782.html \
+ 		test_bug430624.html \
 		test_bug430723.html \
 		test_child.html \
 		test_grandchild.html \

docshell/test/navigation/test_bug430624.html

@@ -0,0 +1,55 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=430624
+-->
+<head>
+  <title>Test for Bug 430624</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>  
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=430624">Mozilla Bug 430624</a>
+<p id="display"></p>
+
+
+
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 430624 **/
+
+function onLoad() {
+  window.frames[0].frameElement.onload = onReload;
+  window.frames[0].location = window.frames[0].location;
+}
+
+function onReload() {
+  var bodyElement = window.frames[0].frameElement.contentDocument.body;
+  sendChar('S', bodyElement);
+  sendChar('t', bodyElement);
+  sendChar('i', bodyElement);
+  sendChar('l', bodyElement);
+  sendChar('l', bodyElement);
+  sendChar(' ', bodyElement);
+
+  is(bodyElement.innerHTML, "Still contentEditable", "Check we're contentEditable after reload");
+
+  SimpleTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+
+<iframe onload="onLoad()" src="data:text/html;charset=utf-8,<body contenteditable>contentEditable</body>"></iframe>
+
+</body>
+</html>
+

editor/libeditor/base/crashtests/430624-1.html

@@ -0,0 +1,14 @@
+<html>
+<head>
+<script>
+function crash() {
+  window.frames[0].onload = null;
+  window.frames[0].location = 'data:text/html;charset=utf-8,2nd%20page';
+}
+</script>
+</head>
+<body onload="crash()">
+  <!-- iframe contents: <html><body onload="document.body.setAttribute('spellcheck', true);"></body></html> -->
+  <iframe src="data:text/html;charset=utf-8;base64,PGh0bWw%2BPGJvZHkgb25sb2FkPSJkb2N1bWVudC5ib2R5LnNldEF0dHJpYnV0ZSgnc3BlbGxjaGVjaycsIHRydWUpOyI%2BPC9ib2R5PjwvaHRtbD4%3D"></iframe>
+</body>
+</html>

editor/libeditor/base/crashtests/crashtests.list

@@ -1,3 +1,4 @@
 load 382527-1.html
 load 402172-1.html
 load 407256-1.html
+load 430624-1.html