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+
authorEhsan Akhgari <ehsan@mozilla.com>
Thu, 18 Nov 2010 16:01:12 -0500
changeset 57958 64fdcad8cb1111c18e75face950d9c707b2b5673
parent 57957 91980a82eeae105a7e64d2c73fe23f9b97a3a90f
child 57959 406748270073ffdf35ccb1a203943751c38a4a37
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersbzbarsky, blocking-beta8
bugs612447
milestone2.0b8pre
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
editor/composer/src/nsEditingSession.cpp
editor/composer/src/nsEditingSession.h
editor/libeditor/html/nsHTMLEditor.cpp
editor/libeditor/html/nsHTMLEditor.h
editor/libeditor/html/tests/Makefile.in
editor/libeditor/html/tests/test_bug612447.html
--- a/content/base/src/nsFrameLoader.cpp
+++ b/content/base/src/nsFrameLoader.cpp
@@ -82,16 +82,18 @@
 #include "nsDOMError.h"
 #include "nsGUIEvent.h"
 #include "nsEventDispatcher.h"
 #include "nsISHistory.h"
 #include "nsISHistoryInternal.h"
 #include "nsIDocShellHistory.h"
 #include "nsIDOMNSHTMLDocument.h"
 #include "nsIXULWindow.h"
+#include "nsIEditor.h"
+#include "nsIEditorDocShell.h"
 
 #include "nsLayoutUtils.h"
 #include "nsIView.h"
 #include "nsPLDOMEvent.h"
 
 #include "nsIURI.h"
 #include "nsIURL.h"
 #include "nsNetUtil.h"
@@ -681,16 +683,23 @@ nsFrameLoader::Show(PRInt32 marginWidth,
     nsCOMPtr<nsIDOMNSHTMLDocument> doc =
       do_QueryInterface(presShell->GetDocument());
 
     if (doc) {
       nsAutoString designMode;
       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"));
       }
     }
   }
 
   mInShow = PR_FALSE;
   if (mHideCalled) {
--- a/editor/composer/src/nsEditingSession.cpp
+++ b/editor/composer/src/nsEditingSession.cpp
@@ -446,18 +446,23 @@ nsEditingSession::SetupEditorOnWindow(ns
     NS_ENSURE_SUCCESS(rv, rv);
     utils->SetImageAnimationMode(imgIContainer::kDontAnimMode);
   }
 
   // create and set editor
   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);
 
   // setup the HTML editor command controller
   if (needHTMLController)
   {
     // The third controller takes an nsIEditor as the context
--- a/editor/composer/src/nsEditingSession.h
+++ b/editor/composer/src/nsEditingSession.h
@@ -163,13 +163,16 @@ protected:
   PRUint32        mEditorFlags;
   PRUint32        mEditorStatus;
   PRUint32        mBaseCommandControllerId;
   PRUint32        mDocStateControllerId;
   PRUint32        mHTMLCommandControllerId;
 
   // Make sure the docshell we use is safe
   nsWeakPtr       mDocShell;
+
+  // See if we can reuse an existing editor
+  nsWeakPtr       mExistingEditor;
 };
 
 
 
 #endif // nsEditingSession_h__
--- a/editor/libeditor/html/nsHTMLEditor.cpp
+++ b/editor/libeditor/html/nsHTMLEditor.cpp
@@ -139,19 +139,17 @@ nsresult NS_NewTextEditRules(nsIEditRule
 nsresult NS_NewHTMLEditRules(nsIEditRules** aInstancePtrResult);
 
 #define IsLinkTag(s) (s.EqualsIgnoreCase(hrefText))
 #define IsNamedAnchorTag(s) (s.EqualsIgnoreCase(anchorTxt) || s.EqualsIgnoreCase(namedanchorText))
 
 nsHTMLEditor::nsHTMLEditor()
 : nsPlaintextEditor()
 , mIgnoreSpuriousDragEvent(PR_FALSE)
-, mTypeInState(nsnull)
 , mCRInParagraphCreatesParagraph(PR_FALSE)
-, mHTMLCSSUtils(nsnull)
 , mSelectedCellIndex(0)
 , mIsObjectResizingEnabled(PR_TRUE)
 , mIsResizing(PR_FALSE)
 , mIsAbsolutelyPositioningEnabled(PR_TRUE)
 , mResizedObjectIsAbsolutelyPositioned(PR_FALSE)
 , mGrabberClicked(PR_FALSE)
 , mIsMoving(PR_FALSE)
 , mSnapToGridEnabled(PR_FALSE)
@@ -196,21 +194,19 @@ nsHTMLEditor::~nsHTMLEditor()
     }
     listener = do_QueryInterface(mSelectionListenerP);
     if (listener)
     {
       selPriv->RemoveSelectionListener(listener); 
     }
   }
 
-  NS_IF_RELEASE(mTypeInState);
+  mTypeInState = nsnull;
   mSelectionListenerP = nsnull;
 
-  delete mHTMLCSSUtils;
-
   // free any default style propItems
   RemoveAllDefaultProperties();
 
   while (mStyleSheetURLs.Length())
   {
     RemoveOverrideStyleSheet(mStyleSheetURLs[0]);
   }
 
@@ -283,35 +279,32 @@ nsHTMLEditor::Init(nsIDOMDocument *aDoc,
     // disable Composer-only features
     if (IsMailEditor())
     {
       SetAbsolutePositioningEnabled(PR_FALSE);
       SetSnapToGridEnabled(PR_FALSE);
     }
 
     // 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);
 
     // disable links
     nsPresContext *context = aPresShell->GetPresContext();
     NS_ENSURE_TRUE(context, NS_ERROR_NULL_POINTER);
     if (!IsPlaintextEditor() && !IsInteractionAllowed()) {
       mLinkHandler = context->GetLinkHandler();
 
       context->SetLinkHandler(nsnull);
     }
 
     // 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);
     if (!mSelectionListenerP) {return NS_ERROR_NULL_POINTER;}
 
     if (!IsInteractionAllowed()) {
       // ignore any errors from this in case the file is missing
       AddOverrideStyleSheet(NS_LITERAL_STRING("resource://gre/res/EditorOverride.css"));
@@ -429,17 +422,19 @@ nsHTMLEditor::FindSelectionRoot(nsINode 
   // containing aContent.
   content = content->GetEditingHost();
   return content.forget();
 }
 
 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);
   return NS_OK;
 }
 
 nsresult
 nsHTMLEditor::InstallEventListeners()
--- a/editor/libeditor/html/nsHTMLEditor.h
+++ b/editor/libeditor/html/nsHTMLEditor.h
@@ -776,22 +776,22 @@ protected:
                                    PRBool aDeleteSelection,
                                    PRBool aTrustedInput);
 
 // Data members
 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;
 
   nsString mLastStyleSheetURL;
   nsString mLastOverrideStyleSheetURL;
 
   // Maintain a list of associated style sheets and their urls.
--- a/editor/libeditor/html/tests/Makefile.in
+++ b/editor/libeditor/html/tests/Makefile.in
@@ -63,16 +63,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug537046.html \
 		test_bug550434.html \
 		test_bug551704.html \
 		test_bug592592.html \
 		test_bug597784.html \
 		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 \
 		test_htmleditor_keyevent_handling.html \
 		test_select_all_without_body.html \
 		file_select_all_without_body.html \
 		test_root_element_replacement.html \
 		$(NULL)
new file mode 100644
--- /dev/null
+++ b/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>