Bug 1460509 - part 2: Make TextEditRules::CreateBR() and TextEditRules::CreateMozBR() return both new <br> element node and error code since if they cause destroying the editor, each caller needs NS_ERROR_EDITOR_DESTROYED result r?m_kato
First, this patch changes TextEditRules::CreateBRInternal() to a private method
for making any callers use CreateBR() or CreateMozBR() instead.
Then, this patch makes TextEditRules::CreateBRInternal() return both nsresult
and created <br> element with CreateElementResult class.
Finally, this patch makes all callers of them check if they don't return an
error code including NS_ERROR_EDITOR_DESTROYED.
MozReview-Commit-ID: 18OvPmbDVHK
--- a/editor/libeditor/EditorUtils.h
+++ b/editor/libeditor/EditorUtils.h
@@ -21,16 +21,21 @@ class nsAtom;
class nsIContentIterator;
class nsISimpleEnumerator;
class nsITransferable;
class nsRange;
namespace mozilla {
template <class T> class OwningNonNull;
+namespace dom {
+class Element;
+class Text;
+} // namespace dom
+
/***************************************************************************
* EditActionResult is useful to return multiple results of an editor
* action handler without out params.
* Note that when you return an anonymous instance from a method, you should
* use EditActionIgnored(), EditActionHandled() or EditActionCanceled() for
* easier to read. In other words, EditActionResult should be used when
* declaring return type of a method, being an argument or defined as a local
* variable.
@@ -141,16 +146,71 @@ EditActionHandled(nsresult aRv = NS_OK)
*/
inline EditActionResult
EditActionCanceled(nsresult aRv = NS_OK)
{
return EditActionResult(aRv, true, true);
}
/***************************************************************************
+ * CreateNodeResultBase is a simple class for CreateSomething() methods
+ * which want to return new node.
+ */
+template<typename NodeType>
+class CreateNodeResultBase;
+
+typedef CreateNodeResultBase<dom::Element> CreateElementResult;
+
+template<typename NodeType>
+class MOZ_STACK_CLASS CreateNodeResultBase final
+{
+ typedef CreateNodeResultBase<NodeType> SelfType;
+public:
+ bool Succeeded() const { return NS_SUCCEEDED(mRv); }
+ bool Failed() const { return NS_FAILED(mRv); }
+ nsresult Rv() const { return mRv; }
+ NodeType* GetNewNode() const { return mNode; }
+
+ CreateNodeResultBase() = delete;
+
+ explicit CreateNodeResultBase(nsresult aRv)
+ : mRv(aRv)
+ {
+ MOZ_DIAGNOSTIC_ASSERT(NS_FAILED(mRv));
+ }
+
+ explicit CreateNodeResultBase(NodeType* aNode)
+ : mNode(aNode)
+ , mRv(aNode ? NS_OK : NS_ERROR_FAILURE)
+ {
+ }
+
+ explicit CreateNodeResultBase(already_AddRefed<NodeType>&& aNode)
+ : mNode(aNode)
+ , mRv(mNode.get() ? NS_OK : NS_ERROR_FAILURE)
+ {
+ }
+
+ CreateNodeResultBase(const SelfType& aOther) = delete;
+ SelfType& operator=(const SelfType& aOther) = delete;
+ CreateNodeResultBase(SelfType&& aOther) = default;
+ SelfType& operator=(SelfType&& aOther) = default;
+
+ already_AddRefed<NodeType> forget()
+ {
+ mRv = NS_ERROR_NOT_INITIALIZED;
+ return mNode.forget();
+ }
+
+private:
+ RefPtr<NodeType> mNode;
+ nsresult mRv;
+};
+
+/***************************************************************************
* SplitNodeResult is a simple class for
* EditorBase::SplitNodeDeepWithTransaction().
* This makes the callers' code easier to read.
*/
class MOZ_STACK_CLASS SplitNodeResult final
{
public:
bool Succeeded() const { return NS_SUCCEEDED(mRv); }
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -5593,19 +5593,20 @@ HTMLEditRules::WillAlign(const nsAString
}
// Remember our new block for postprocessing
mNewBlock = div;
// Set up the alignment on the div, using HTML or CSS
rv = AlignBlock(*div, aAlignType, ContentsOnly::yes);
NS_ENSURE_SUCCESS(rv, rv);
*aHandled = true;
// Put in a moz-br so that it won't get deleted
- RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(div, 0));
- if (NS_WARN_IF(!brElement)) {
- return NS_ERROR_FAILURE;
+ CreateElementResult createMozBrResult =
+ CreateMozBR(EditorRawDOMPoint(div, 0));
+ if (NS_WARN_IF(createMozBrResult.Failed())) {
+ return createMozBrResult.Rv();
}
EditorRawDOMPoint atStartOfDiv(div, 0);
ErrorResult error;
SelectionRef().Collapse(atStartOfDiv, error);
// Don't restore the selection
selectionRestorer.Abort();
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
@@ -7331,19 +7332,20 @@ HTMLEditRules::ReturnInHeader(Element& a
MOZ_DIAGNOSTIC_ASSERT(
HTMLEditUtils::IsHeader(*prevItem));
bool isEmptyNode;
rv = HTMLEditorRef().IsEmptyNode(prevItem, &isEmptyNode);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
if (isEmptyNode) {
- RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(prevItem, 0));
- if (NS_WARN_IF(!brElement)) {
- return NS_ERROR_FAILURE;
+ CreateElementResult createMozBrResult =
+ CreateMozBR(EditorRawDOMPoint(prevItem, 0));
+ if (NS_WARN_IF(createMozBrResult.Failed())) {
+ return createMozBrResult.Rv();
}
}
}
// If the new (righthand) header node is empty, delete it
if (IsEmptyBlockElement(aHeader, IgnoreSingleBR::eYes)) {
rv = HTMLEditorRef().DeleteNodeWithTransaction(aHeader);
if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -7799,19 +7801,20 @@ HTMLEditRules::ReturnInListItem(Element&
HTMLEditorRef().GetPriorHTMLSibling(&aListItem);
if (prevItem && HTMLEditUtils::IsListItem(prevItem)) {
bool isEmptyNode;
rv = HTMLEditorRef().IsEmptyNode(prevItem, &isEmptyNode);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
if (isEmptyNode) {
- RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(prevItem, 0));
- if (NS_WARN_IF(!brElement)) {
- return NS_ERROR_FAILURE;
+ CreateElementResult createMozBrResult =
+ CreateMozBR(EditorRawDOMPoint(prevItem, 0));
+ if (NS_WARN_IF(createMozBrResult.Failed())) {
+ return createMozBrResult.Rv();
}
} else {
rv = HTMLEditorRef().IsEmptyNode(&aListItem, &isEmptyNode, true);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
if (isEmptyNode) {
RefPtr<nsAtom> nodeAtom = aListItem.NodeInfo()->NameAtom();
@@ -8553,18 +8556,20 @@ HTMLEditRules::AdjustSpecialBreaks()
// Put moz-br's into these empty li's and td's
for (auto& node : nodeArray) {
// Need to put br at END of node. It may have empty containers in it and
// still pass the "IsEmptyNode" test, and we want the br's to be after
// them. Also, we want the br to be after the selection if the selection
// is in this node.
EditorRawDOMPoint endOfNode;
endOfNode.SetToEndOf(node);
- RefPtr<Element> brElement = CreateMozBR(endOfNode);
- if (NS_WARN_IF(!brElement)) {
+ // XXX This method should return nsreuslt due to may be destroyed by this
+ // CreateMozBr() call.
+ CreateElementResult createMozBrResult = CreateMozBR(endOfNode);
+ if (NS_WARN_IF(createMozBrResult.Failed())) {
return;
}
}
}
nsresult
HTMLEditRules::AdjustWhitespace()
{
@@ -8777,19 +8782,19 @@ HTMLEditRules::AdjustSelection(nsIEditor
if (point.GetContainer() == rootElement) {
// Our root node is completely empty. Don't add a <br> here.
// AfterEditInner() will add one for us when it calls
// CreateBogusNodeIfNeeded()!
return NS_OK;
}
// we know we can skip the rest of this routine given the cirumstance
- RefPtr<Element> brElement = CreateMozBR(point);
- if (NS_WARN_IF(!brElement)) {
- return NS_ERROR_FAILURE;
+ CreateElementResult createMozBrResult = CreateMozBR(point);
+ if (NS_WARN_IF(createMozBrResult.Failed())) {
+ return createMozBrResult.Rv();
}
return NS_OK;
}
}
// are we in a text node?
if (point.IsInTextNode()) {
return NS_OK; // we LIKE it when we are in a text node. that RULZ
@@ -8807,21 +8812,21 @@ HTMLEditRules::AdjustSelection(nsIEditor
RefPtr<Element> block = HTMLEditorRef().GetBlock(*point.GetContainer());
RefPtr<Element> nearBlock = HTMLEditorRef().GetBlockNodeParent(nearNode);
if (block && block == nearBlock) {
if (nearNode && TextEditUtils::IsBreak(nearNode)) {
if (!HTMLEditorRef().IsVisibleBRElement(nearNode)) {
// need to insert special moz BR. Why? Because if we don't
// the user will see no new line for the break. Also, things
// like table cells won't grow in height.
- RefPtr<Element> brElement = CreateMozBR(point);
- if (NS_WARN_IF(!brElement)) {
- return NS_ERROR_FAILURE;
+ CreateElementResult createMozBrResult = CreateMozBR(point);
+ if (NS_WARN_IF(createMozBrResult.Failed())) {
+ return createMozBrResult.Rv();
}
- point.Set(brElement);
+ point.Set(createMozBrResult.GetNewNode());
// selection stays *before* moz-br, sticking to it
ErrorResult error;
SelectionRef().SetInterlinePosition(true, error);
NS_WARNING_ASSERTION(!error.Failed(),
"Failed to set interline position");
error = NS_OK;
SelectionRef().Collapse(point, error);
if (NS_WARN_IF(error.Failed())) {
@@ -9445,20 +9450,21 @@ HTMLEditRules::InsertBRIfNeededInternal(
nsresult rv = HTMLEditorRef().IsEmptyNode(&aNode, &isEmpty);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
if (!isEmpty) {
return NS_OK;
}
- RefPtr<Element> brElement =
- CreateBRInternal(EditorRawDOMPoint(&aNode, 0), aInsertMozBR);
- if (NS_WARN_IF(!brElement)) {
- return NS_ERROR_FAILURE;
+ CreateElementResult createBrResult =
+ !aInsertMozBR ? CreateBR(EditorRawDOMPoint(&aNode, 0)) :
+ CreateMozBR(EditorRawDOMPoint(&aNode, 0));
+ if (NS_WARN_IF(createBrResult.Failed())) {
+ return createBrResult.Rv();
}
return NS_OK;
}
void
HTMLEditRules::DidCreateNode(Selection& aSelection,
Element& aNewElement)
{
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -39,16 +39,23 @@
#include "nsUnicharUtils.h"
#include "nsIHTMLCollection.h"
#include "nsPrintfCString.h"
namespace mozilla {
using namespace dom;
+template CreateElementResult
+TextEditRules::CreateBRInternal(const EditorDOMPoint& aPointToInsert,
+ bool aCreateMozBR);
+template CreateElementResult
+TextEditRules::CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
+ bool aCreateMozBR);
+
#define CANCEL_OPERATION_IF_READONLY_OR_DISABLED \
if (IsReadonly() || IsDisabled()) \
{ \
*aCancel = true; \
return NS_OK; \
};
/********************************************************
@@ -1374,19 +1381,19 @@ TextEditRules::CreateTrailingBRIfNeeded(
if (NS_WARN_IF(!lastChild)) {
return NS_ERROR_FAILURE;
}
if (!lastChild->IsHTMLElement(nsGkAtoms::br)) {
AutoTransactionsConserveSelection dontChangeMySelection(&TextEditorRef());
EditorRawDOMPoint endOfRoot;
endOfRoot.SetToEndOf(rootElement);
- RefPtr<Element> brElement = CreateMozBR(endOfRoot);
- if (NS_WARN_IF(!brElement)) {
- return NS_ERROR_FAILURE;
+ CreateElementResult createMozBrResult = CreateMozBR(endOfRoot);
+ if (NS_WARN_IF(createMozBrResult.Failed())) {
+ return createMozBrResult.Rv();
}
return NS_OK;
}
// Check to see if the trailing BR is a former bogus node - this will have
// stuck around if we previously morphed a trailing node into a bogus node.
if (!TextEditorRef().IsMozEditorBogusNode(lastChild)) {
return NS_OK;
@@ -1658,47 +1665,56 @@ TextEditRules::FillBufWithPWChars(nsAStr
char16_t passwordChar = LookAndFeel::GetPasswordCharacter();
aOutString->Truncate();
for (int32_t i = 0; i < aLength; i++) {
aOutString->Append(passwordChar);
}
}
-already_AddRefed<Element>
-TextEditRules::CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
- bool aCreateMozBR)
+template<typename PT, typename CT>
+CreateElementResult
+TextEditRules::CreateBRInternal(
+ const EditorDOMPointBase<PT, CT>& aPointToInsert,
+ bool aCreateMozBR)
{
MOZ_ASSERT(IsEditorDataAvailable());
if (NS_WARN_IF(!aPointToInsert.IsSet())) {
- return nullptr;
+ return CreateElementResult(NS_ERROR_FAILURE);
}
RefPtr<Element> brElement =
TextEditorRef().InsertBrElementWithTransaction(SelectionRef(),
aPointToInsert);
+ if (NS_WARN_IF(!CanHandleEditAction())) {
+ return CreateElementResult(NS_ERROR_EDITOR_DESTROYED);
+ }
if (NS_WARN_IF(!brElement)) {
- return nullptr;
+ return CreateElementResult(NS_ERROR_FAILURE);
}
// give it special moz attr
- if (aCreateMozBR) {
- // XXX Why do we need to set this attribute with transaction?
- nsresult rv =
- TextEditorRef().SetAttributeWithTransaction(*brElement, *nsGkAtoms::type,
- NS_LITERAL_STRING("_moz"));
- if (NS_WARN_IF(NS_FAILED(rv))) {
- // XXX Don't we need to remove the new <br> element from the DOM tree
- // in this case?
- return nullptr;
- }
+ if (!aCreateMozBR) {
+ return CreateElementResult(brElement.forget());
}
- return brElement.forget();
+ // XXX Why do we need to set this attribute with transaction?
+ nsresult rv =
+ TextEditorRef().SetAttributeWithTransaction(*brElement, *nsGkAtoms::type,
+ NS_LITERAL_STRING("_moz"));
+ // XXX Don't we need to remove the new <br> element from the DOM tree
+ // in these case?
+ if (NS_WARN_IF(!CanHandleEditAction())) {
+ return CreateElementResult(NS_ERROR_EDITOR_DESTROYED);
+ }
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ return CreateElementResult(NS_ERROR_FAILURE);
+ }
+ return CreateElementResult(brElement.forget());
}
nsresult
TextEditRules::DocumentModified()
{
return NS_ERROR_NOT_IMPLEMENTED;
}
--- a/editor/libeditor/TextEditRules.h
+++ b/editor/libeditor/TextEditRules.h
@@ -3,16 +3,17 @@
* 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 mozilla_TextEditRules_h
#define mozilla_TextEditRules_h
#include "mozilla/EditAction.h"
#include "mozilla/EditorDOMPoint.h"
+#include "mozilla/EditorUtils.h"
#include "mozilla/HTMLEditor.h" // for nsIEditor::AsHTMLEditor()
#include "mozilla/TextEditor.h"
#include "nsCOMPtr.h"
#include "nsCycleCollectionParticipant.h"
#include "nsIEditor.h"
#include "nsINamed.h"
#include "nsISupportsImpl.h"
#include "nsITimer.h"
@@ -204,53 +205,55 @@ protected:
*/
void RemoveIMETextFromPWBuf(uint32_t& aStart, nsAString* aIMEString);
/**
* Create a normal <br> element and insert it to aPointToInsert.
*
* @param aPointToInsert The point where the new <br> element will be
* inserted.
- * @return Returns created <br> element.
+ * @return Returns created <br> element or an error code
+ * if couldn't create new <br> element.
*/
template<typename PT, typename CT>
- already_AddRefed<Element>
+ CreateElementResult
CreateBR(const EditorDOMPointBase<PT, CT>& aPointToInsert)
{
- return CreateBRInternal(aPointToInsert, false);
+ CreateElementResult ret = CreateBRInternal(aPointToInsert, false);
+#ifdef DEBUG
+ // If editor is destroyed, it must return NS_ERROR_EDITOR_DESTROYED.
+ if (!CanHandleEditAction()) {
+ MOZ_ASSERT(ret.Rv() == NS_ERROR_EDITOR_DESTROYED);
+ }
+#endif // #ifdef DEBUG
+ return ret;
}
/**
* Create a moz-<br> element and insert it to aPointToInsert.
*
* @param aPointToInsert The point where the new moz-<br> element will be
* inserted.
- * @return Returns created moz-<br> element.
+ * @return Returns created <br> element or an error code
+ * if couldn't create new <br> element.
*/
template<typename PT, typename CT>
- already_AddRefed<Element>
+ CreateElementResult
CreateMozBR(const EditorDOMPointBase<PT, CT>& aPointToInsert)
{
- return CreateBRInternal(aPointToInsert, true);
+ CreateElementResult ret = CreateBRInternal(aPointToInsert, true);
+#ifdef DEBUG
+ // If editor is destroyed, it must return NS_ERROR_EDITOR_DESTROYED.
+ if (!CanHandleEditAction()) {
+ MOZ_ASSERT(ret.Rv() == NS_ERROR_EDITOR_DESTROYED);
+ }
+#endif // #ifdef DEBUG
+ return ret;
}
- /**
- * Create a normal <br> element or a moz-<br> element and insert it to
- * aPointToInsert.
- *
- * @param aParentToInsert The point where the new <br> element will be
- * inserted.
- * @param aCreateMozBR true if the caller wants to create a moz-<br>
- * element. Otherwise, false.
- * @return Returns created <br> element.
- */
- already_AddRefed<Element>
- CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
- bool aCreateMozBR);
-
void UndefineCaretBidiLevel();
nsresult CheckBidiLevelForDeletion(const EditorRawDOMPoint& aSelectionPoint,
nsIEditor::EDirection aAction,
bool* aCancel);
nsresult HideLastPWInput();
@@ -262,16 +265,32 @@ protected:
bool IsReadonly() const;
bool IsDisabled() const;
bool IsMailEditor() const;
bool DontEchoPassword() const;
private:
TextEditor* MOZ_NON_OWNING_REF mTextEditor;
+ /**
+ * Create a normal <br> element or a moz-<br> element and insert it to
+ * aPointToInsert.
+ *
+ * @param aParentToInsert The point where the new <br> element will be
+ * inserted.
+ * @param aCreateMozBR true if the caller wants to create a moz-<br>
+ * element. Otherwise, false.
+ * @return Returns created <br> element and error code.
+ * If it succeeded, never returns nullptr.
+ */
+ template<typename PT, typename CT>
+ CreateElementResult
+ CreateBRInternal(const EditorDOMPointBase<PT, CT>& aPointToInsert,
+ bool aCreateMozBR);
+
protected:
/**
* AutoSafeEditorData grabs editor instance and related instances during
* handling an edit action. When this is created, its pointer is set to
* the mSafeData, and this guarantees the lifetime of grabbing objects
* until it's destroyed.
*/
class MOZ_STACK_CLASS AutoSafeEditorData