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

--HG--
extra : rebase_source : 328a91146539763cec317105b92aa3ce5d4b8233
This commit is contained in:
Masayuki Nakano 2018-05-11 15:52:24 +09:00
parent 9ac857639c
commit b2f86bbbe1
4 changed files with 162 additions and 61 deletions

View File

@ -26,6 +26,11 @@ 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.
@ -145,6 +150,61 @@ 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().

View File

@ -5598,9 +5598,10 @@ HTMLEditRules::WillAlign(const nsAString& aAlignType,
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;
@ -7336,9 +7337,10 @@ HTMLEditRules::ReturnInHeader(Element& aHeader,
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();
}
}
}
@ -7804,9 +7806,10 @@ HTMLEditRules::ReturnInListItem(Element& aListItem,
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);
@ -8558,8 +8561,10 @@ HTMLEditRules::AdjustSpecialBreaks()
// 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;
}
}
@ -8782,9 +8787,9 @@ HTMLEditRules::AdjustSelection(nsIEditor::EDirection aAction)
}
// 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;
}
@ -8812,11 +8817,11 @@ HTMLEditRules::AdjustSelection(nsIEditor::EDirection aAction)
// 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);
@ -9450,10 +9455,11 @@ HTMLEditRules::InsertBRIfNeededInternal(nsINode& aNode,
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;
}

View File

@ -44,6 +44,13 @@ 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()) \
{ \
@ -1379,9 +1386,9 @@ TextEditRules::CreateTrailingBRIfNeeded()
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;
}
@ -1663,37 +1670,46 @@ TextEditRules::FillBufWithPWChars(nsAString* aOutString,
}
}
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

View File

@ -8,6 +8,7 @@
#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"
@ -209,13 +210,21 @@ protected:
*
* @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;
}
/**
@ -223,29 +232,23 @@ protected:
*
* @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,
@ -267,6 +270,22 @@ protected:
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