Bug 1371657 - Ensure we use the right root element in editor. r=masayuki, a=lizzard
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Wed, 30 Aug 2017 20:43:12 -0400
changeset 423887 3b007282412b7030ccef0d45e7b72890caa00b7a
parent 423886 9796d42a13d4d39d6ab6f0bdd53ee616a8ddaaa7
child 423888 044d1a40524824a2394164b9a3ed95ccb877b76a
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki, lizzard
bugs1371657
milestone56.0
Bug 1371657 - Ensure we use the right root element in editor. r=masayuki, a=lizzard
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -269,16 +269,20 @@ HTMLEditor::Init(nsIDOMDocument* aDoc,
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     // Init mutation observer
     nsCOMPtr<nsINode> document = do_QueryInterface(aDoc);
     document->AddMutationObserverUnlessExists(this);
 
+    if (!mRootElement) {
+      UpdateRootElement();
+    }
+
     // disable Composer-only features
     if (IsMailEditor()) {
       SetAbsolutePositioningEnabled(false);
       SetSnapToGridEnabled(false);
     }
 
     // Init the HTML-CSS utils
     mCSSEditUtils = MakeUnique<CSSEditUtils>(this);
@@ -340,58 +344,37 @@ HTMLEditor::PreDestroy(bool aDestroyingF
 
   // Clean up after our anonymous content -- we don't want these nodes to
   // stay around (which they would, since the frames have an owning reference).
   HideAnonymousEditingUIs();
 
   return TextEditor::PreDestroy(aDestroyingFrames);
 }
 
-NS_IMETHODIMP
-HTMLEditor::GetRootElement(nsIDOMElement** aRootElement)
+void
+HTMLEditor::UpdateRootElement()
 {
-  NS_ENSURE_ARG_POINTER(aRootElement);
-
-  if (mRootElement) {
-    return EditorBase::GetRootElement(aRootElement);
-  }
-
-  *aRootElement = nullptr;
-
   // Use the HTML documents body element as the editor root if we didn't
   // get a root element during initialization.
 
   nsCOMPtr<nsIDOMElement> rootElement;
   nsCOMPtr<nsIDOMHTMLElement> bodyElement;
-  nsresult rv = GetBodyElement(getter_AddRefs(bodyElement));
-  NS_ENSURE_SUCCESS(rv, rv);
-
+  GetBodyElement(getter_AddRefs(bodyElement));
   if (bodyElement) {
     rootElement = bodyElement;
   } else {
     // If there is no HTML body element,
     // we should use the document root element instead.
     nsCOMPtr<nsIDOMDocument> domDocument = GetDOMDocument();
-    if (NS_WARN_IF(!domDocument)) {
-      return NS_ERROR_NOT_INITIALIZED;
-    }
-    rv = domDocument->GetDocumentElement(getter_AddRefs(rootElement));
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    // Document can have no elements
-    if (!rootElement) {
-      return NS_ERROR_NOT_AVAILABLE;
+    if (domDocument) {
+      domDocument->GetDocumentElement(getter_AddRefs(rootElement));
     }
   }
 
   mRootElement = do_QueryInterface(rootElement);
-  rootElement.forget(aRootElement);
-
-  return NS_OK;
 }
 
 already_AddRefed<nsIContent>
 HTMLEditor::FindSelectionRoot(nsINode* aNode)
 {
   NS_PRECONDITION(aNode->IsNodeOfType(nsINode::eDOCUMENT) ||
                   aNode->IsNodeOfType(nsINode::eCONTENT),
                   "aNode must be content or document node");
@@ -3269,20 +3252,21 @@ HTMLEditor::DoContentInserted(nsIDocumen
 {
   if (!IsInObservedSubtree(aDocument, aContainer, aChild)) {
     return;
   }
 
   nsCOMPtr<nsIHTMLEditor> kungFuDeathGrip(this);
 
   if (ShouldReplaceRootElement()) {
+    UpdateRootElement();
     nsContentUtils::AddScriptRunner(
-      NewRunnableMethod("HTMLEditor::ResetRootElementAndEventTarget",
+      NewRunnableMethod("HTMLEditor::NotifyRootChanged",
                         this,
-                        &HTMLEditor::ResetRootElementAndEventTarget));
+                        &HTMLEditor::NotifyRootChanged));
   }
   // We don't need to handle our own modifications
   else if (!mAction && (aContainer ? aContainer->IsEditable() : aDocument->IsEditable())) {
     if (IsMozEditorBogusNode(aChild)) {
       // Ignore insertion of the bogus node
       return;
     }
     // Protect the edit rules object from dying
@@ -3319,20 +3303,21 @@ HTMLEditor::ContentRemoved(nsIDocument* 
 {
   if (!IsInObservedSubtree(aDocument, aContainer, aChild)) {
     return;
   }
 
   nsCOMPtr<nsIHTMLEditor> kungFuDeathGrip(this);
 
   if (SameCOMIdentity(aChild, mRootElement)) {
+    mRootElement = nullptr;
     nsContentUtils::AddScriptRunner(
-      NewRunnableMethod("HTMLEditor::ResetRootElementAndEventTarget",
+      NewRunnableMethod("HTMLEditor::NotifyRootChanged",
                         this,
-                        &HTMLEditor::ResetRootElementAndEventTarget));
+                        &HTMLEditor::NotifyRootChanged));
   }
   // We don't need to handle our own modifications
   else if (!mAction && (aContainer ? aContainer->IsEditable() : aDocument->IsEditable())) {
     if (aChild && IsMozEditorBogusNode(aChild)) {
       // Ignore removal of the bogus node
       return;
     }
     // Protect the edit rules object from dying
@@ -5089,34 +5074,28 @@ HTMLEditor::ShouldReplaceRootElement()
   // If we temporary set document root element to mRootElement, but there is
   // body element now, we should replace the root element by the body element.
   nsCOMPtr<nsIDOMHTMLElement> docBody;
   GetBodyElement(getter_AddRefs(docBody));
   return !SameCOMIdentity(docBody, mRootElement);
 }
 
 void
-HTMLEditor::ResetRootElementAndEventTarget()
+HTMLEditor::NotifyRootChanged()
 {
   nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
 
-  // Need to remove the event listeners first because BeginningOfDocument
-  // could set a new root (and event target is set by InstallEventListeners())
-  // and we won't be able to remove them from the old event target then.
   RemoveEventListeners();
-  mRootElement = nullptr;
   nsresult rv = InstallEventListeners();
   if (NS_FAILED(rv)) {
     return;
   }
 
-  // We must have mRootElement now.
-  nsCOMPtr<nsIDOMElement> root;
-  rv = GetRootElement(getter_AddRefs(root));
-  if (NS_FAILED(rv) || !mRootElement) {
+  UpdateRootElement();
+  if (!mRootElement) {
     return;
   }
 
   rv = BeginningOfDocument();
   if (NS_FAILED(rv)) {
     return;
   }
 
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -291,18 +291,16 @@ public:
                                   nsIDocument* aDoc) override;
   NS_IMETHOD_(bool) IsModifiableNode(nsIDOMNode* aNode) override;
   virtual bool IsModifiableNode(nsINode* aNode) override;
 
   NS_IMETHOD GetIsSelectionEditable(bool* aIsSelectionEditable) override;
 
   NS_IMETHOD SelectAll() override;
 
-  NS_IMETHOD GetRootElement(nsIDOMElement** aRootElement) override;
-
   // nsICSSLoaderObserver
   NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet,
                               bool aWasAlternate, nsresult aStatus) override;
 
   // Utility Routines, not part of public API
   NS_IMETHOD TypedText(const nsAString& aString,
                        ETypingAction aAction) override;
   nsresult InsertNodeAtPoint(nsIDOMNode* aNode,
@@ -414,17 +412,17 @@ protected:
 
   NS_IMETHOD InitRules() override;
 
   virtual void CreateEventListeners() override;
   virtual nsresult InstallEventListeners() override;
   virtual void RemoveEventListeners() override;
 
   bool ShouldReplaceRootElement();
-  void ResetRootElementAndEventTarget();
+  void NotifyRootChanged();
   nsresult GetBodyElement(nsIDOMHTMLElement** aBody);
 
   /**
    * Get the focused node of this editor.
    * @return    If the editor has focus, this returns the focused node.
    *            Otherwise, returns null.
    */
   already_AddRefed<nsINode> GetFocusedNode();
@@ -877,16 +875,18 @@ protected:
                                     int32_t& aBorderTop,
                                     int32_t& aMarginLeft,
                                     int32_t& aMarginTop);
 
   bool IsInObservedSubtree(nsIDocument* aDocument,
                            nsIContent* aContainer,
                            nsIContent* aChild);
 
+  void UpdateRootElement();
+
   // resizing
   bool mIsObjectResizingEnabled;
   bool mIsResizing;
   bool mPreserveRatio;
   bool mResizedObjectIsAnImage;
 
   // absolute positioning
   bool mIsAbsolutelyPositioningEnabled;