Bug 1056166 part 3 - Clean up ChangeAttributeTxn; r=ehsan
authorAryeh Gregor <ayg@aryeh.name>
Fri, 29 Aug 2014 14:43:23 +0300
changeset 223970 4ce8c3c09faec4038ea5c2ad7d5b8e9e1dde97ee
parent 223969 a3ce25fc058d0e6730b5e8c5fe27b83f2a3eb275
child 223971 3abc849c2d320560a400ece9c2128e52383895ea
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1056166
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1056166 part 3 - Clean up ChangeAttributeTxn; r=ehsan
editor/libeditor/ChangeAttributeTxn.cpp
editor/libeditor/ChangeAttributeTxn.h
editor/libeditor/nsEditor.cpp
editor/libeditor/nsEditor.h
--- a/editor/libeditor/ChangeAttributeTxn.cpp
+++ b/editor/libeditor/ChangeAttributeTxn.cpp
@@ -1,109 +1,92 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "mozilla/dom/Element.h"        // for Element
+
 #include "ChangeAttributeTxn.h"
 #include "nsAString.h"
-#include "nsDebug.h"                    // for NS_ASSERTION
 #include "nsError.h"                    // for NS_ERROR_NOT_INITIALIZED, etc
-#include "nsIDOMElement.h"              // for nsIDOMElement
-#include "nsIEditor.h"                  // for nsIEditor
-#include "nsString.h"                   // for nsString
+
+using namespace mozilla;
+using namespace mozilla::dom;
 
-ChangeAttributeTxn::ChangeAttributeTxn()
+ChangeAttributeTxn::ChangeAttributeTxn(Element& aElement, nsIAtom& aAttribute,
+                                       const nsAString* aValue)
   : EditTxn()
+  , mElement(&aElement)
+  , mAttribute(&aAttribute)
+  , mValue(aValue ? *aValue : EmptyString())
+  , mRemoveAttribute(!aValue)
+  , mAttributeWasSet(false)
+  , mUndoValue()
 {
 }
 
 ChangeAttributeTxn::~ChangeAttributeTxn()
 {
 }
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(ChangeAttributeTxn, EditTxn,
                                    mElement)
 
 NS_IMPL_ADDREF_INHERITED(ChangeAttributeTxn, EditTxn)
 NS_IMPL_RELEASE_INHERITED(ChangeAttributeTxn, EditTxn)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ChangeAttributeTxn)
 NS_INTERFACE_MAP_END_INHERITING(EditTxn)
 
-NS_IMETHODIMP ChangeAttributeTxn::Init(nsIEditor      *aEditor,
-                                       nsIDOMElement  *aElement,
-                                       const nsAString& aAttribute,
-                                       const nsAString& aValue,
-                                       bool aRemoveAttribute)
+NS_IMETHODIMP
+ChangeAttributeTxn::DoTransaction()
 {
-  NS_ASSERTION(aEditor && aElement, "bad arg");
-  if (!aEditor || !aElement) { return NS_ERROR_NULL_POINTER; }
+  // Need to get the current value of the attribute and save it, and set
+  // mAttributeWasSet
+  mAttributeWasSet = mElement->GetAttr(kNameSpaceID_None, mAttribute,
+                                       mUndoValue);
 
-  mEditor = aEditor;
-  mElement = do_QueryInterface(aElement);
-  mAttribute = aAttribute;
-  mValue = aValue;
-  mRemoveAttribute = aRemoveAttribute;
-  mAttributeWasSet=false;
-  mUndoValue.Truncate();
-  return NS_OK;
+  // XXX: hack until attribute-was-set code is implemented
+  if (!mUndoValue.IsEmpty()) {
+    mAttributeWasSet = true;
+  }
+  // XXX: end hack
+
+  // Now set the attribute to the new value
+  if (mRemoveAttribute) {
+    return mElement->UnsetAttr(kNameSpaceID_None, mAttribute, true);
+  }
+
+  return mElement->SetAttr(kNameSpaceID_None, mAttribute, mValue, true);
 }
 
-NS_IMETHODIMP ChangeAttributeTxn::DoTransaction(void)
+NS_IMETHODIMP
+ChangeAttributeTxn::UndoTransaction()
 {
-  NS_ASSERTION(mEditor && mElement, "bad state");
-  if (!mEditor || !mElement) { return NS_ERROR_NOT_INITIALIZED; }
-
-  // need to get the current value of the attribute and save it, and set mAttributeWasSet
-  nsresult result = mEditor->GetAttributeValue(mElement, mAttribute, mUndoValue, &mAttributeWasSet);
-  // XXX: hack until attribute-was-set code is implemented
-  if (!mUndoValue.IsEmpty())
-    mAttributeWasSet = true;
-  // XXX: end hack
-  
-  // now set the attribute to the new value
-  if (!mRemoveAttribute)
-    result = mElement->SetAttribute(mAttribute, mValue);
-  else
-    result = mElement->RemoveAttribute(mAttribute);
-
-  return result;
+  if (mAttributeWasSet) {
+    return mElement->SetAttr(kNameSpaceID_None, mAttribute, mUndoValue, true);
+  }
+  return mElement->UnsetAttr(kNameSpaceID_None, mAttribute, true);
 }
 
-NS_IMETHODIMP ChangeAttributeTxn::UndoTransaction(void)
+NS_IMETHODIMP
+ChangeAttributeTxn::RedoTransaction()
 {
-  NS_ASSERTION(mEditor && mElement, "bad state");
-  if (!mEditor || !mElement) { return NS_ERROR_NOT_INITIALIZED; }
+  if (mRemoveAttribute) {
+    return mElement->UnsetAttr(kNameSpaceID_None, mAttribute, true);
+  }
 
-  nsresult result;
-  if (mAttributeWasSet)
-    result = mElement->SetAttribute(mAttribute, mUndoValue);
-  else
-    result = mElement->RemoveAttribute(mAttribute);
-
-  return result;
+  return mElement->SetAttr(kNameSpaceID_None, mAttribute, mValue, true);
 }
 
-NS_IMETHODIMP ChangeAttributeTxn::RedoTransaction(void)
-{
-  NS_ASSERTION(mEditor && mElement, "bad state");
-  if (!mEditor || !mElement) { return NS_ERROR_NOT_INITIALIZED; }
-
-  nsresult result;
-  if (!mRemoveAttribute)
-    result = mElement->SetAttribute(mAttribute, mValue);
-  else
-    result = mElement->RemoveAttribute(mAttribute);
-
-  return result;
-}
-
-NS_IMETHODIMP ChangeAttributeTxn::GetTxnDescription(nsAString& aString)
+NS_IMETHODIMP
+ChangeAttributeTxn::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("ChangeAttributeTxn: [mRemoveAttribute == ");
 
-  if (!mRemoveAttribute)
+  if (mRemoveAttribute) {
+    aString.AppendLiteral("true] ");
+  } else {
     aString.AppendLiteral("false] ");
-  else
-    aString.AppendLiteral("true] ");
-  aString += mAttribute;
+  }
+  aString += nsDependentAtomString(mAttribute);
   return NS_OK;
 }
--- a/editor/libeditor/ChangeAttributeTxn.h
+++ b/editor/libeditor/ChangeAttributeTxn.h
@@ -1,73 +1,69 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef ChangeAttributeTxn_h__
 #define ChangeAttributeTxn_h__
 
-#include "EditTxn.h"
-#include "nsCOMPtr.h"
-#include "nsCycleCollectionParticipant.h"
-#include "nsIDOMElement.h"
-#include "nsISupportsImpl.h"
-#include "nsString.h"
-#include "nscore.h"
+#include "EditTxn.h"                      // base class
+#include "mozilla/Attributes.h"           // MOZ_OVERRIDE
+#include "nsCOMPtr.h"                     // nsCOMPtr members
+#include "nsCycleCollectionParticipant.h" // NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED
+#include "nsISupportsImpl.h"              // NS_DECL_ISUPPORTS_INHERITED
+#include "nsString.h"                     // nsString members
 
-class nsIEditor;
+class nsIAtom;
+
+namespace mozilla {
+namespace dom {
+
+class Element;
 
 /**
- * A transaction that changes an attribute of a content node. 
- * This transaction covers add, remove, and change attribute.
+ * A transaction that changes an attribute of a content node.  This transaction
+ * covers add, remove, and change attribute.
  */
 class ChangeAttributeTxn : public EditTxn
 {
 public:
-  /** Initialize the transaction.
-    * @param aEditor the object providing core editing operations
-    * @param aNode   the node whose attribute will be changed
+  /** @param aElement the element whose attribute will be changed
     * @param aAttribute the name of the attribute to change
-    * @param aValue     the new value for aAttribute, if aRemoveAttribute is false
-    * @param aRemoveAttribute if true, remove aAttribute from aNode
+    * @param aValue     the new value for aAttribute, or null to remove
     */
-  NS_IMETHOD Init(nsIEditor      *aEditor,
-                  nsIDOMElement  *aNode,
-                  const nsAString& aAttribute,
-                  const nsAString& aValue,
-                  bool aRemoveAttribute);
-
-  ChangeAttributeTxn();
+  ChangeAttributeTxn(Element& aElement, nsIAtom& aAttribute,
+                     const nsAString* aValue);
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ChangeAttributeTxn, EditTxn)
 
   NS_DECL_EDITTXN
 
-  NS_IMETHOD RedoTransaction();
+  NS_IMETHOD RedoTransaction() MOZ_OVERRIDE;
 
-protected:
+private:
   virtual ~ChangeAttributeTxn();
 
-  /** the editor that created this transaction */
-  nsIEditor*  mEditor;
-  
-  /** the element to operate upon */
-  nsCOMPtr<nsIDOMElement> mElement;
-  
-  /** the attribute to change */
-  nsString mAttribute;
+  /** The element to operate upon */
+  nsCOMPtr<Element> mElement;
 
-  /** the value to set the attribute to (ignored if mRemoveAttribute==true) */
+  /** The attribute to change */
+  nsCOMPtr<nsIAtom> mAttribute;
+
+  /** The value to set the attribute to (ignored if mRemoveAttribute==true) */
   nsString mValue;
 
-  /** the value to set the attribute to for undo */
-  nsString mUndoValue;
+  /** True if the operation is to remove mAttribute from mElement */
+  bool mRemoveAttribute;
 
-  /** true if the mAttribute was set on mElement at the time of execution */
-  bool     mAttributeWasSet;
+  /** True if the mAttribute was set on mElement at the time of execution */
+  bool mAttributeWasSet;
 
-  /** true if the operation is to remove mAttribute from mElement */
-  bool     mRemoveAttribute;
+  /** The value to set the attribute to for undo */
+  nsString mUndoValue;
 };
 
+}
+}
+
 #endif
--- a/editor/libeditor/nsEditor.cpp
+++ b/editor/libeditor/nsEditor.cpp
@@ -1169,26 +1169,27 @@ nsEditor::CanPaste(int32_t aSelectionTyp
 }
 
 NS_IMETHODIMP
 nsEditor::CanPasteTransferable(nsITransferable *aTransferable, bool *aCanPaste)
 {
   return NS_ERROR_NOT_IMPLEMENTED; 
 }
 
-NS_IMETHODIMP 
-nsEditor::SetAttribute(nsIDOMElement *aElement, const nsAString & aAttribute, const nsAString & aValue)
-{
-  nsRefPtr<ChangeAttributeTxn> txn;
-  nsresult result = CreateTxnForSetAttribute(aElement, aAttribute, aValue,
-                                             getter_AddRefs(txn));
-  if (NS_SUCCEEDED(result))  {
-    result = DoTransaction(txn);  
-  }
-  return result;
+NS_IMETHODIMP
+nsEditor::SetAttribute(nsIDOMElement* aElement, const nsAString& aAttribute,
+                       const nsAString& aValue)
+{
+  nsCOMPtr<Element> element = do_QueryInterface(aElement);
+  NS_ENSURE_TRUE(element, NS_ERROR_NULL_POINTER);
+  nsCOMPtr<nsIAtom> attribute = do_GetAtom(aAttribute);
+
+  nsRefPtr<ChangeAttributeTxn> txn =
+    CreateTxnForSetAttribute(*element, *attribute, aValue);
+  return DoTransaction(txn);
 }
 
 NS_IMETHODIMP 
 nsEditor::GetAttributeValue(nsIDOMElement *aElement, 
                             const nsAString & aAttribute, 
                             nsAString & aResultValue, 
                             bool *aResultIsSet)
 {
@@ -1202,26 +1203,26 @@ nsEditor::GetAttributeValue(nsIDOMElemen
   NS_ENSURE_SUCCESS(rv, rv);
   if (!DOMStringIsNull(value)) {
     *aResultIsSet = true;
     aResultValue = value;
   }
   return rv;
 }
 
-NS_IMETHODIMP 
-nsEditor::RemoveAttribute(nsIDOMElement *aElement, const nsAString& aAttribute)
-{
-  nsRefPtr<ChangeAttributeTxn> txn;
-  nsresult result = CreateTxnForRemoveAttribute(aElement, aAttribute,
-                                                getter_AddRefs(txn));
-  if (NS_SUCCEEDED(result))  {
-    result = DoTransaction(txn);  
-  }
-  return result;
+NS_IMETHODIMP
+nsEditor::RemoveAttribute(nsIDOMElement* aElement, const nsAString& aAttribute)
+{
+  nsCOMPtr<Element> element = do_QueryInterface(aElement);
+  NS_ENSURE_TRUE(element, NS_ERROR_NULL_POINTER);
+  nsCOMPtr<nsIAtom> attribute = do_GetAtom(aAttribute);
+
+  nsRefPtr<ChangeAttributeTxn> txn =
+    CreateTxnForRemoveAttribute(*element, *attribute);
+  return DoTransaction(txn);
 }
 
 
 bool
 nsEditor::OutputsMozDirty()
 {
   // Return true for Composer (!eEditorAllowInteraction) or mail
   // (eEditorMailMask), but false for webpages.
@@ -4228,52 +4229,34 @@ nsEditor::DoAfterUndoTransaction()
 void
 nsEditor::DoAfterRedoTransaction()
 {
   // all redoable transactions are non-transient
   MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
     IncrementModificationCount(1)));
 }
 
-NS_IMETHODIMP 
-nsEditor::CreateTxnForSetAttribute(nsIDOMElement *aElement, 
-                                   const nsAString& aAttribute, 
-                                   const nsAString& aValue,
-                                   ChangeAttributeTxn ** aTxn)
-{
-  NS_ENSURE_TRUE(aElement, NS_ERROR_NULL_POINTER);
-
-  nsRefPtr<ChangeAttributeTxn> txn = new ChangeAttributeTxn();
-
-  nsresult rv = txn->Init(this, aElement, aAttribute, aValue, false);
-  if (NS_SUCCEEDED(rv))
-  {
-    txn.forget(aTxn);
-  }
-
-  return rv;
-}
-
-
-NS_IMETHODIMP 
-nsEditor::CreateTxnForRemoveAttribute(nsIDOMElement *aElement, 
-                                      const nsAString& aAttribute,
-                                      ChangeAttributeTxn ** aTxn)
-{
-  NS_ENSURE_TRUE(aElement, NS_ERROR_NULL_POINTER);
-
-  nsRefPtr<ChangeAttributeTxn> txn = new ChangeAttributeTxn();
-
-  nsresult rv = txn->Init(this, aElement, aAttribute, EmptyString(), true);
-  if (NS_SUCCEEDED(rv))
-  {
-    txn.forget(aTxn);
-  }
-
-  return rv;
+already_AddRefed<ChangeAttributeTxn>
+nsEditor::CreateTxnForSetAttribute(Element& aElement, nsIAtom& aAttribute,
+                                   const nsAString& aValue)
+{
+  nsRefPtr<ChangeAttributeTxn> txn =
+    new ChangeAttributeTxn(aElement, aAttribute, &aValue);
+
+  return txn.forget();
+}
+
+
+already_AddRefed<ChangeAttributeTxn>
+nsEditor::CreateTxnForRemoveAttribute(Element& aElement, nsIAtom& aAttribute)
+{
+  nsRefPtr<ChangeAttributeTxn> txn =
+    new ChangeAttributeTxn(aElement, aAttribute, nullptr);
+
+  return txn.forget();
 }
 
 
 already_AddRefed<CreateElementTxn>
 nsEditor::CreateTxnForCreateElement(nsIAtom& aTag,
                                     nsINode& aParent,
                                     int32_t aPosition)
 {
--- a/editor/libeditor/nsEditor.h
+++ b/editor/libeditor/nsEditor.h
@@ -23,17 +23,16 @@
 #include "nsIWeakReferenceUtils.h"      // for nsWeakPtr
 #include "nsLiteralString.h"            // for NS_LITERAL_STRING
 #include "nsSelectionState.h"           // for nsRangeUpdater, etc
 #include "nsString.h"                   // for nsCString
 #include "nsWeakReference.h"            // for nsSupportsWeakReference
 #include "nscore.h"                     // for nsresult, nsAString, etc
 
 class AddStyleSheetTxn;
-class ChangeAttributeTxn;
 class DeleteNodeTxn;
 class EditAggregateTxn;
 class IMETextTxn;
 class JoinElementTxn;
 class RemoveStyleSheetTxn;
 class SplitElementTxn;
 class nsIAtom;
 class nsIContent;
@@ -63,16 +62,17 @@ class nsString;
 class nsTransactionManager;
 
 namespace mozilla {
 class CSSStyleSheet;
 class ErrorResult;
 class TextComposition;
 
 namespace dom {
+class ChangeAttributeTxn;
 class CreateElementTxn;
 class DataTransfer;
 class DeleteTextTxn;
 class Element;
 class EventTarget;
 class InsertTextTxn;
 class InsertNodeTxn;
 class Selection;
@@ -249,28 +249,29 @@ public:
   void EndIMEComposition();
 
   void SwitchTextDirectionTo(uint32_t aDirection);
 
 protected:
   nsresult DetermineCurrentDirection();
   void FireInputEvent();
 
-  /** create a transaction for setting aAttribute to aValue on aElement
+  /** Create a transaction for setting aAttribute to aValue on aElement.  Never
+    * returns null.
     */
-  NS_IMETHOD CreateTxnForSetAttribute(nsIDOMElement *aElement,
-                                      const nsAString &  aAttribute,
-                                      const nsAString &  aValue,
-                                      ChangeAttributeTxn ** aTxn);
+  already_AddRefed<mozilla::dom::ChangeAttributeTxn>
+  CreateTxnForSetAttribute(mozilla::dom::Element& aElement,
+                           nsIAtom& aAttribute, const nsAString& aValue);
 
-  /** create a transaction for removing aAttribute on aElement
+  /** Create a transaction for removing aAttribute on aElement.  Never returns
+    * null.
     */
-  NS_IMETHOD CreateTxnForRemoveAttribute(nsIDOMElement *aElement,
-                                         const nsAString &  aAttribute,
-                                         ChangeAttributeTxn ** aTxn);
+  already_AddRefed<mozilla::dom::ChangeAttributeTxn>
+  CreateTxnForRemoveAttribute(mozilla::dom::Element& aElement,
+                              nsIAtom& aAttribute);
 
   /** create a transaction for creating a new child node of aParent of type aTag.
     */
   already_AddRefed<mozilla::dom::CreateElementTxn>
   CreateTxnForCreateElement(nsIAtom& aTag,
                             nsINode& aParent,
                             int32_t aPosition);