Bug 612447 - Don't Recreate an editor object attached to a document in a frame if that frame is restyled; r=bzbarsky a=blocking-beta8+

content/base/src/nsFrameLoader.cpp

@@ -87,6 +87,8 @@
 #include "nsIDocShellHistory.h"
 #include "nsIDOMNSHTMLDocument.h"
 #include "nsIXULWindow.h"
+#include "nsIEditor.h"
+#include "nsIEditorDocShell.h"
 
 #include "nsLayoutUtils.h"
 #include "nsIView.h"
@@ -686,6 +688,13 @@ nsFrameLoader::Show(PRInt32 marginWidth, PRInt32 marginHeight,
       doc->GetDesignMode(designMode);
 
       if (designMode.EqualsLiteral("on")) {
+        // Hold on to the editor object to let the document reattach to the
+        // same editor object, instead of creating a new one.
+        nsCOMPtr<nsIEditorDocShell> editorDocshell = do_QueryInterface(mDocShell);
+        nsCOMPtr<nsIEditor> editor;
+        nsresult rv = editorDocshell->GetEditor(getter_AddRefs(editor));
+        NS_ENSURE_SUCCESS(rv, PR_FALSE);
+
         doc->SetDesignMode(NS_LITERAL_STRING("off"));
         doc->SetDesignMode(NS_LITERAL_STRING("on"));
       }

editor/composer/src/nsEditingSession.cpp

@@ -451,8 +451,13 @@ nsEditingSession::SetupEditorOnWindow(nsIDOMWindow *aWindow)
   nsCOMPtr<nsIEditorDocShell> editorDocShell = do_QueryInterface(docShell, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsCOMPtr<nsIEditor> editor = do_CreateInstance(classString, &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
+  // Try to reuse an existing editor
+  nsCOMPtr<nsIEditor> editor = do_QueryReferent(mExistingEditor);
+  if (!editor) {
+    editor = do_CreateInstance(classString, &rv);
+    NS_ENSURE_SUCCESS(rv, rv);
+    mExistingEditor = do_GetWeakReference(editor);
+  }
   // set the editor on the docShell. The docShell now owns it.
   rv = editorDocShell->SetEditor(editor);
   NS_ENSURE_SUCCESS(rv, rv);

editor/composer/src/nsEditingSession.h

@@ -168,6 +168,9 @@ protected:
 
   // Make sure the docshell we use is safe
   nsWeakPtr       mDocShell;
+
+  // See if we can reuse an existing editor
+  nsWeakPtr       mExistingEditor;
 };
 
 

editor/libeditor/html/nsHTMLEditor.cpp

@@ -144,9 +144,7 @@ nsresult NS_NewHTMLEditRules(nsIEditRules** aInstancePtrResult);
 nsHTMLEditor::nsHTMLEditor()
 : nsPlaintextEditor()
 , mIgnoreSpuriousDragEvent(PR_FALSE)
-, mTypeInState(nsnull)
 , mCRInParagraphCreatesParagraph(PR_FALSE)
-, mHTMLCSSUtils(nsnull)
 , mSelectedCellIndex(0)
 , mIsObjectResizingEnabled(PR_TRUE)
 , mIsResizing(PR_FALSE)
@@ -201,11 +199,9 @@ nsHTMLEditor::~nsHTMLEditor()
     }
   }
 
-  NS_IF_RELEASE(mTypeInState);
+  mTypeInState = nsnull;
   mSelectionListenerP = nsnull;
 
-  delete mHTMLCSSUtils;
-
   // free any default style propItems
   RemoveAllDefaultProperties();
 
@@ -288,9 +284,7 @@ nsHTMLEditor::Init(nsIDOMDocument *aDoc, nsIPresShell *aPresShell,
     }
 
     // Init the HTML-CSS utils
-    if (mHTMLCSSUtils)
-      delete mHTMLCSSUtils;
-    result = NS_NewHTMLCSSUtils(&mHTMLCSSUtils);
+    result = NS_NewHTMLCSSUtils(getter_Transfers(mHTMLCSSUtils));
     if (NS_FAILED(result)) { return result; }
     mHTMLCSSUtils->Init(this);
 
@@ -306,7 +300,6 @@ nsHTMLEditor::Init(nsIDOMDocument *aDoc, nsIPresShell *aPresShell,
     // init the type-in state
     mTypeInState = new TypeInState();
     if (!mTypeInState) {return NS_ERROR_NULL_POINTER;}
-    NS_ADDREF(mTypeInState);
 
     // init the selection listener for image resizing
     mSelectionListenerP = new ResizerSelectionListener(this);
@@ -434,7 +427,9 @@ nsHTMLEditor::FindSelectionRoot(nsINode *aNode)
 nsresult
 nsHTMLEditor::CreateEventListeners()
 {
-  NS_ENSURE_TRUE(!mEventListener, NS_ERROR_ALREADY_INITIALIZED);
+  // Don't create the handler twice
+  if (mEventListener)
+    return NS_OK;
   mEventListener = do_QueryInterface(
     static_cast<nsIDOMKeyListener*>(new nsHTMLEditorEventListener()));
   NS_ENSURE_TRUE(mEventListener, NS_ERROR_OUT_OF_MEMORY);

editor/libeditor/html/nsHTMLEditor.h

@@ -781,12 +781,12 @@ protected:
 
   nsCOMArray<nsIContentFilter> mContentFilters;
 
-  TypeInState*         mTypeInState;
+  nsRefPtr<TypeInState>        mTypeInState;
 
   PRPackedBool mCRInParagraphCreatesParagraph;
 
   PRPackedBool mCSSAware;
-  nsHTMLCSSUtils *mHTMLCSSUtils;
+  nsAutoPtr<nsHTMLCSSUtils> mHTMLCSSUtils;
 
   // Used by GetFirstSelectedCell and GetNextSelectedCell
   PRInt32  mSelectedCellIndex;

editor/libeditor/html/tests/Makefile.in

@@ -68,6 +68,7 @@ _TEST_FILES = \
 		test_bug599322.html \
 		test_bug607584.html \
 		test_bug611182.html \
+		test_bug612447.html \
 		test_CF_HTML_clipboard.html \
 		test_contenteditable_focus.html \
 		test_contenteditable_text_input_handling.html \

editor/libeditor/html/tests/test_bug612447.html

@@ -0,0 +1,75 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=612447
+-->
+<head>
+  <title>Test for Bug 612447</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/WindowSnapshot.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=612447">Mozilla Bug 612447</a>
+<p id="display"></p>
+<div id="content">
+<iframe></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 612447 **/
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(function() {
+  function editorCommandsEnabled() {
+    var caught = false;
+    try {
+      doc.execCommand("justifyfull", false, null);
+    } catch (e) {
+      caught = true;
+    }
+    return !caught;
+  }
+
+  var i = document.querySelector("iframe");
+  var doc = i.contentDocument;
+  var win = i.contentWindow;
+  var b = doc.body;
+  doc.designMode = "on";
+  i.focus();
+  b.focus();
+  var beforeA = snapshotWindow(win, true);
+  synthesizeKey("X", {});
+  var beforeB = snapshotWindow(win, true);
+  is(b.textContent, "X", "Typing should work");
+  while (b.firstChild) {
+    b.removeChild(b.firstChild);
+  }
+  ok(editorCommandsEnabled(), "The editor commands should work");
+
+  i.style.display = "block";
+  document.clientWidth;
+
+  i.focus();
+  b.focus();
+  var afterA = snapshotWindow(win, true);
+  synthesizeKey("X", {});
+  var afterB = snapshotWindow(win, true);
+  is(b.textContent, "X", "Typing should work");
+  while (b.firstChild) {
+    b.removeChild(b.firstChild);
+  }
+  ok(editorCommandsEnabled(), "The editor commands should work");
+
+  ok(compareSnapshots(beforeA, afterA, true)[0], "The iframes should look the same before typing");
+  ok(compareSnapshots(beforeB, afterB, true)[0], "The iframes should look the same after typing");
+
+  SimpleTest.finish();
+});
+
+</script>
+</pre>
+</body>
+</html>