Bug 1425412 - part 5: Create some factory methods of DeleteTextTransaction and remove EditorBase::CreateTxnForDeleteText() and EditorBase::CreateTxnForDeleteCharacter() r?m_kato
DeleteTextTransaction should have 3 factory methods. One is, simply to create
an instance with a pair of offset and length. The others are, to create an
instance for deleting a previous or next character at offset.
The former was EditorBase::CreateTxnForDeleteText() and the latter was
EditorBase::CreateTxnForDeleteCharacter(), but this patch creates
DeleteTextTransaction::MaybeCreate() for the former,
DeleteTextTransaction::MaybeCreateForPreviousCharacter() and
DeleteTextTransaction::MaybeCreateForNextCharacter() for the latter.
MozReview-Commit-ID: DFELbmAJDo3
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -155,21 +155,21 @@ DeleteRangeTransaction::CreateTxnsToDele
numToDel = aEnd.Offset() - aStart.Offset();
MOZ_DIAGNOSTIC_ASSERT(numToDel > 0);
}
RefPtr<nsGenericDOMDataNode> charDataNode =
static_cast<nsGenericDOMDataNode*>(aStart.Container());
RefPtr<DeleteTextTransaction> deleteTextTransaction =
- new DeleteTextTransaction(*mEditorBase, *charDataNode, aStart.Offset(),
- numToDel, mRangeUpdater);
+ DeleteTextTransaction::MaybeCreate(*mEditorBase, *charDataNode,
+ aStart.Offset(), numToDel);
// If the text node isn't editable, it should be never undone/redone.
// So, the transaction shouldn't be recorded.
- if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
+ if (NS_WARN_IF(!deleteTextTransaction)) {
return NS_ERROR_FAILURE;
}
AppendChild(deleteTextTransaction);
return NS_OK;
}
// Even if we detect invalid range, we should ignore it for removing
// specified range's nodes as far as possible.
@@ -219,21 +219,21 @@ DeleteRangeTransaction::CreateTxnsToDele
if (!numToDelete) {
return NS_OK;
}
RefPtr<nsGenericDOMDataNode> dataNode =
static_cast<nsGenericDOMDataNode*>(aPoint.Container());
RefPtr<DeleteTextTransaction> deleteTextTransaction =
- new DeleteTextTransaction(*mEditorBase, *dataNode, startOffset, numToDelete,
- mRangeUpdater);
+ DeleteTextTransaction::MaybeCreate(*mEditorBase, *dataNode,
+ startOffset, numToDelete);
// If the text node isn't editable, it should be never undone/redone.
// So, the transaction shouldn't be recorded.
- if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
+ if (NS_WARN_IF(!deleteTextTransaction)) {
return NS_ERROR_FAILURE;
}
AppendChild(deleteTextTransaction);
return NS_OK;
}
nsresult
--- a/editor/libeditor/DeleteTextTransaction.cpp
+++ b/editor/libeditor/DeleteTextTransaction.cpp
@@ -2,41 +2,105 @@
/* 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 "DeleteTextTransaction.h"
#include "mozilla/Assertions.h"
#include "mozilla/EditorBase.h"
+#include "mozilla/EditorDOMPoint.h"
#include "mozilla/SelectionState.h"
#include "mozilla/dom/Selection.h"
#include "nsDebug.h"
#include "nsError.h"
#include "nsIEditor.h"
#include "nsISupportsImpl.h"
#include "nsAString.h"
namespace mozilla {
using namespace dom;
+// static
+already_AddRefed<DeleteTextTransaction>
+DeleteTextTransaction::MaybeCreate(EditorBase& aEditorBase,
+ nsGenericDOMDataNode& aCharData,
+ uint32_t aOffset,
+ uint32_t aLengthToDelete)
+{
+ RefPtr<DeleteTextTransaction> transaction =
+ new DeleteTextTransaction(aEditorBase, aCharData, aOffset, aLengthToDelete);
+ return transaction.forget();
+}
+
+// static
+already_AddRefed<DeleteTextTransaction>
+DeleteTextTransaction::MaybeCreateForPreviousCharacter(
+ EditorBase& aEditorBase,
+ nsGenericDOMDataNode& aCharData,
+ uint32_t aOffset)
+{
+ if (NS_WARN_IF(!aOffset)) {
+ return nullptr;
+ }
+
+ nsAutoString data;
+ aCharData.GetData(data);
+ if (NS_WARN_IF(data.IsEmpty())) {
+ return nullptr;
+ }
+
+ uint32_t length = 1;
+ uint32_t offset = aOffset - 1;
+ if (offset &&
+ NS_IS_LOW_SURROGATE(data[offset]) &&
+ NS_IS_HIGH_SURROGATE(data[offset - 1])) {
+ ++length;
+ --offset;
+ }
+ return DeleteTextTransaction::MaybeCreate(aEditorBase, aCharData,
+ offset, length);
+}
+
+// static
+already_AddRefed<DeleteTextTransaction>
+DeleteTextTransaction::MaybeCreateForNextCharacter(
+ EditorBase& aEditorBase,
+ nsGenericDOMDataNode& aCharData,
+ uint32_t aOffset)
+{
+ nsAutoString data;
+ aCharData.GetData(data);
+ if (NS_WARN_IF(aOffset >= data.Length()) ||
+ NS_WARN_IF(data.IsEmpty())) {
+ return nullptr;
+ }
+
+ uint32_t length = 1;
+ if (aOffset + 1 < data.Length() &&
+ NS_IS_HIGH_SURROGATE(data[aOffset]) &&
+ NS_IS_LOW_SURROGATE(data[aOffset + 1])) {
+ ++length;
+ }
+ return DeleteTextTransaction::MaybeCreate(aEditorBase, aCharData,
+ aOffset, length);
+}
+
DeleteTextTransaction::DeleteTextTransaction(
EditorBase& aEditorBase,
nsGenericDOMDataNode& aCharData,
uint32_t aOffset,
- uint32_t aNumCharsToDelete,
- RangeUpdater* aRangeUpdater)
+ uint32_t aLengthToDelete)
: mEditorBase(&aEditorBase)
, mCharData(&aCharData)
, mOffset(aOffset)
- , mNumCharsToDelete(aNumCharsToDelete)
- , mRangeUpdater(aRangeUpdater)
+ , mLengthToDelete(aLengthToDelete)
{
- NS_ASSERTION(mCharData->Length() >= aOffset + aNumCharsToDelete,
+ NS_ASSERTION(mCharData->Length() >= aOffset + aLengthToDelete,
"Trying to delete more characters than in node");
}
NS_IMPL_CYCLE_COLLECTION_INHERITED(DeleteTextTransaction, EditTransactionBase,
mEditorBase,
mCharData)
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DeleteTextTransaction)
@@ -54,34 +118,38 @@ DeleteTextTransaction::CanDoIt() const
NS_IMETHODIMP
DeleteTextTransaction::DoTransaction()
{
if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mCharData)) {
return NS_ERROR_NOT_AVAILABLE;
}
// Get the text that we're about to delete
- nsresult rv = mCharData->SubstringData(mOffset, mNumCharsToDelete,
+ nsresult rv = mCharData->SubstringData(mOffset, mLengthToDelete,
mDeletedText);
MOZ_ASSERT(NS_SUCCEEDED(rv));
- rv = mCharData->DeleteData(mOffset, mNumCharsToDelete);
- NS_ENSURE_SUCCESS(rv, rv);
+ rv = mCharData->DeleteData(mOffset, mLengthToDelete);
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ return rv;
+ }
- if (mRangeUpdater) {
- mRangeUpdater->SelAdjDeleteText(mCharData, mOffset, mNumCharsToDelete);
- }
+ mEditorBase->RangeUpdaterRef().
+ SelAdjDeleteText(mCharData, mOffset, mLengthToDelete);
// Only set selection to deletion point if editor gives permission
if (mEditorBase->GetShouldTxnSetSelection()) {
RefPtr<Selection> selection = mEditorBase->GetSelection();
- NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
- rv = selection->Collapse(mCharData, mOffset);
- NS_ASSERTION(NS_SUCCEEDED(rv),
- "Selection could not be collapsed after undo of deletetext");
- NS_ENSURE_SUCCESS(rv, rv);
+ if (NS_WARN_IF(!selection)) {
+ return NS_ERROR_FAILURE;
+ }
+ ErrorResult error;
+ selection->Collapse(EditorRawDOMPoint(mCharData, mOffset), error);
+ if (NS_WARN_IF(error.Failed())) {
+ return error.StealNSResult();
+ }
}
// Else do nothing - DOM Range gravity will adjust selection
return NS_OK;
}
//XXX: We may want to store the selection state and restore it properly. Was
// it an insertion point or an extended selection?
NS_IMETHODIMP
--- a/editor/libeditor/DeleteTextTransaction.h
+++ b/editor/libeditor/DeleteTextTransaction.h
@@ -19,62 +19,84 @@ namespace mozilla {
class EditorBase;
class RangeUpdater;
/**
* A transaction that removes text from a content node.
*/
class DeleteTextTransaction final : public EditTransactionBase
{
-public:
- /**
- * Initialize the transaction.
- * @param aEditorBase The provider of basic editing operations.
- * @param aElement The content node to remove text from.
- * @param aOffset The location in aElement to begin the deletion.
- * @param aNumCharsToDelete The number of characters to delete. Not the
- * number of bytes!
- */
+protected:
DeleteTextTransaction(EditorBase& aEditorBase,
nsGenericDOMDataNode& aCharData,
uint32_t aOffset,
- uint32_t aNumCharsToDelete,
- RangeUpdater* aRangeUpdater);
+ uint32_t aLengthToDelete);
+
+public:
+
+ /**
+ * Creates a delete text transaction to remove given range. This returns
+ * nullptr if it cannot modify the text node.
+ *
+ * @param aEditorBase The provider of basic editing operations.
+ * @param aCharData The content node to remove text from.
+ * @param aOffset The location in aElement to begin the deletion.
+ * @param aLenthToDelete The length to delete.
+ */
+ static already_AddRefed<DeleteTextTransaction>
+ MaybeCreate(EditorBase& aEditorBase,
+ nsGenericDOMDataNode& aCharData,
+ uint32_t aOffset,
+ uint32_t aLengthToDelete);
+
+ /**
+ * Creates a delete text transaction to remove a previous or next character.
+ * Those methods MAY return nullptr.
+ *
+ * @param aEditorBase The provider of basic editing operations.
+ * @param aCharData The content node to remove text from.
+ * @param aOffset The location in aElement to begin the deletion.
+ */
+ static already_AddRefed<DeleteTextTransaction>
+ MaybeCreateForPreviousCharacter(EditorBase& aEditorBase,
+ nsGenericDOMDataNode& aCharData,
+ uint32_t aOffset);
+ static already_AddRefed<DeleteTextTransaction>
+ MaybeCreateForNextCharacter(EditorBase& aEditorBase,
+ nsGenericDOMDataNode& aCharData,
+ uint32_t aOffset);
/**
* CanDoIt() returns true if there are enough members and can modify the
* text. Otherwise, false.
*/
bool CanDoIt() const;
NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(DeleteTextTransaction,
EditTransactionBase)
NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
NS_DECL_EDITTRANSACTIONBASE
- uint32_t GetOffset() { return mOffset; }
+ uint32_t Offset() { return mOffset; }
- uint32_t GetNumCharsToDelete() { return mNumCharsToDelete; }
+ uint32_t LengthToDelete() { return mLengthToDelete; }
protected:
// The provider of basic editing operations.
RefPtr<EditorBase> mEditorBase;
// The CharacterData node to operate upon.
RefPtr<nsGenericDOMDataNode> mCharData;
// The offset into mCharData where the deletion is to take place.
uint32_t mOffset;
- // The number of characters to delete.
- uint32_t mNumCharsToDelete;
+ // The length to delete.
+ uint32_t mLengthToDelete;
// The text that was deleted.
nsString mDeletedText;
-
- // Range updater object.
- RangeUpdater* mRangeUpdater;
};
} // namespace mozilla
#endif // #ifndef DeleteTextTransaction_h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -3001,18 +3001,20 @@ EditorBase::SetTextImpl(Selection& aSele
}
nsresult
EditorBase::DeleteText(nsGenericDOMDataNode& aCharData,
uint32_t aOffset,
uint32_t aLength)
{
RefPtr<DeleteTextTransaction> transaction =
- CreateTxnForDeleteText(aCharData, aOffset, aLength);
- NS_ENSURE_STATE(transaction);
+ DeleteTextTransaction::MaybeCreate(*this, aCharData, aOffset, aLength);
+ if (NS_WARN_IF(!transaction)) {
+ return NS_ERROR_FAILURE;
+ }
AutoRules beginRulesSniffing(this, EditAction::deleteText,
nsIEditor::ePrevious);
// Let listeners know what's up
{
AutoActionListenerArray listeners(mActionListeners);
for (auto& listener : listeners) {
@@ -3032,32 +3034,16 @@ EditorBase::DeleteText(nsGenericDOMDataN
static_cast<nsIDOMCharacterData*>(GetAsDOMNode(&aCharData)), aOffset,
aLength, rv);
}
}
return rv;
}
-already_AddRefed<DeleteTextTransaction>
-EditorBase::CreateTxnForDeleteText(nsGenericDOMDataNode& aCharData,
- uint32_t aOffset,
- uint32_t aLength)
-{
- RefPtr<DeleteTextTransaction> deleteTextTransaction =
- new DeleteTextTransaction(*this, aCharData, aOffset, aLength,
- &mRangeUpdater);
- // If it's not editable, the transaction shouldn't be recorded since it
- // should never be undone/redone.
- if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
- return nullptr;
- }
- return deleteTextTransaction.forget();
-}
-
already_AddRefed<SplitNodeTransaction>
EditorBase::CreateTxnForSplitNode(const EditorRawDOMPoint& aStartOfRightNode)
{
MOZ_ASSERT(aStartOfRightNode.IsSetAndValid());
RefPtr<SplitNodeTransaction> transaction =
new SplitNodeTransaction(*this, aStartOfRightNode);
return transaction.forget();
}
@@ -4723,50 +4709,16 @@ EditorBase::CreateTxnForDeleteSelection(
}
aggregateTransaction->AppendChild(deleteRangeTransaction);
}
}
return aggregateTransaction.forget();
}
-already_AddRefed<DeleteTextTransaction>
-EditorBase::CreateTxnForDeleteCharacter(nsGenericDOMDataNode& aData,
- uint32_t aOffset,
- EDirection aDirection)
-{
- NS_ASSERTION(aDirection == eNext || aDirection == ePrevious,
- "Invalid direction");
- nsAutoString data;
- aData.GetData(data);
- NS_ASSERTION(data.Length(), "Trying to delete from a zero-length node");
- NS_ENSURE_TRUE(data.Length(), nullptr);
-
- uint32_t segOffset = aOffset, segLength = 1;
- if (aDirection == eNext) {
- if (segOffset + 1 < data.Length() &&
- NS_IS_HIGH_SURROGATE(data[segOffset]) &&
- NS_IS_LOW_SURROGATE(data[segOffset+1])) {
- // Delete both halves of the surrogate pair
- ++segLength;
- }
- } else if (aOffset > 0) {
- --segOffset;
- if (segOffset > 0 &&
- NS_IS_LOW_SURROGATE(data[segOffset]) &&
- NS_IS_HIGH_SURROGATE(data[segOffset-1])) {
- ++segLength;
- --segOffset;
- }
- } else {
- return nullptr;
- }
- return CreateTxnForDeleteText(aData, segOffset, segLength);
-}
-
//XXX: currently, this doesn't handle edge conditions because GetNext/GetPrior
//are not implemented
already_AddRefed<EditTransactionBase>
EditorBase::CreateTxnForDeleteRange(nsRange* aRangeToDelete,
EDirection aAction,
nsINode** aRemovingNode,
int32_t* aOffset,
int32_t* aLength)
@@ -4809,22 +4761,23 @@ EditorBase::CreateTxnForDeleteRange(nsRa
RefPtr<nsGenericDOMDataNode> priorNodeAsCharData =
static_cast<nsGenericDOMDataNode*>(priorNode.get());
uint32_t length = priorNode->Length();
// Bail out for empty chardata XXX: Do we want to do something else?
if (NS_WARN_IF(!length)) {
return nullptr;
}
RefPtr<DeleteTextTransaction> deleteTextTransaction =
- CreateTxnForDeleteCharacter(*priorNodeAsCharData, length, ePrevious);
+ DeleteTextTransaction::MaybeCreateForPreviousCharacter(
+ *this, *priorNodeAsCharData, length);
if (NS_WARN_IF(!deleteTextTransaction)) {
return nullptr;
}
- *aOffset = deleteTextTransaction->GetOffset();
- *aLength = deleteTextTransaction->GetNumCharsToDelete();
+ *aOffset = deleteTextTransaction->Offset();
+ *aLength = deleteTextTransaction->LengthToDelete();
priorNode.forget(aRemovingNode);
return deleteTextTransaction.forget();
}
// priorNode is not chardata, so tell its parent to delete it
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
CreateTxnForDeleteNode(priorNode);
if (NS_WARN_IF(!deleteNodeTransaction)) {
@@ -4848,47 +4801,55 @@ EditorBase::CreateTxnForDeleteRange(nsRa
RefPtr<nsGenericDOMDataNode> nextNodeAsCharData =
static_cast<nsGenericDOMDataNode*>(nextNode.get());
uint32_t length = nextNode->Length();
// Bail out for empty chardata XXX: Do we want to do something else?
if (NS_WARN_IF(!length)) {
return nullptr;
}
RefPtr<DeleteTextTransaction> deleteTextTransaction =
- CreateTxnForDeleteCharacter(*nextNodeAsCharData, 0, eNext);
+ DeleteTextTransaction::MaybeCreateForNextCharacter(
+ *this, *nextNodeAsCharData, 0);
if (NS_WARN_IF(!deleteTextTransaction)) {
return nullptr;
}
- *aOffset = deleteTextTransaction->GetOffset();
- *aLength = deleteTextTransaction->GetNumCharsToDelete();
+ *aOffset = deleteTextTransaction->Offset();
+ *aLength = deleteTextTransaction->LengthToDelete();
nextNode.forget(aRemovingNode);
return deleteTextTransaction.forget();
}
// nextNode is not chardata, so tell its parent to delete it
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
CreateTxnForDeleteNode(nextNode);
if (NS_WARN_IF(!deleteNodeTransaction)) {
return nullptr;
}
nextNode.forget(aRemovingNode);
return deleteNodeTransaction.forget();
}
if (node->IsNodeOfType(nsINode::eDATA_NODE)) {
+ if (NS_WARN_IF(aAction != ePrevious && aAction != eNext)) {
+ return nullptr;
+ }
RefPtr<nsGenericDOMDataNode> nodeAsCharData =
static_cast<nsGenericDOMDataNode*>(node.get());
- // we have chardata, so delete a char at the proper offset
+ // We have chardata, so delete a char at the proper offset
RefPtr<DeleteTextTransaction> deleteTextTransaction =
- CreateTxnForDeleteCharacter(*nodeAsCharData, offset, aAction);
+ aAction == ePrevious ?
+ DeleteTextTransaction::MaybeCreateForPreviousCharacter(
+ *this, *nodeAsCharData, offset) :
+ DeleteTextTransaction::MaybeCreateForNextCharacter(
+ *this, *nodeAsCharData, offset);
if (NS_WARN_IF(!deleteTextTransaction)) {
return nullptr;
}
- *aOffset = deleteTextTransaction->GetOffset();
- *aLength = deleteTextTransaction->GetNumCharsToDelete();
+ *aOffset = deleteTextTransaction->Offset();
+ *aLength = deleteTextTransaction->LengthToDelete();
node.forget(aRemovingNode);
return deleteTextTransaction.forget();
}
// we're either deleting a node or chardata, need to dig into the next/prev
// node to find out
nsCOMPtr<nsINode> selectedNode;
if (aAction == ePrevious) {
@@ -4909,31 +4870,37 @@ EditorBase::CreateTxnForDeleteRange(nsRa
}
}
if (NS_WARN_IF(!selectedNode)) {
return nullptr;
}
if (selectedNode->IsNodeOfType(nsINode::eDATA_NODE)) {
+ if (NS_WARN_IF(aAction != ePrevious && aAction != eNext)) {
+ return nullptr;
+ }
RefPtr<nsGenericDOMDataNode> selectedNodeAsCharData =
static_cast<nsGenericDOMDataNode*>(selectedNode.get());
// we are deleting from a chardata node, so do a character deletion
uint32_t position = 0;
if (aAction == ePrevious) {
position = selectedNode->Length();
}
RefPtr<DeleteTextTransaction> deleteTextTransaction =
- CreateTxnForDeleteCharacter(*selectedNodeAsCharData, position,
- aAction);
+ aAction == ePrevious ?
+ DeleteTextTransaction::MaybeCreateForPreviousCharacter(
+ *this, *selectedNodeAsCharData, position) :
+ DeleteTextTransaction::MaybeCreateForNextCharacter(
+ *this, *selectedNodeAsCharData, position);
if (NS_WARN_IF(!deleteTextTransaction)) {
return nullptr;
}
- *aOffset = deleteTextTransaction->GetOffset();
- *aLength = deleteTextTransaction->GetNumCharsToDelete();
+ *aOffset = deleteTextTransaction->Offset();
+ *aLength = deleteTextTransaction->LengthToDelete();
selectedNode.forget(aRemovingNode);
return deleteTextTransaction.forget();
}
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
CreateTxnForDeleteNode(selectedNode);
if (NS_WARN_IF(!deleteNodeTransaction)) {
return nullptr;
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -591,24 +591,16 @@ protected:
* Create a transaction for removing a style sheet.
*/
already_AddRefed<mozilla::RemoveStyleSheetTransaction>
CreateTxnForRemoveStyleSheet(StyleSheet* aSheet);
nsresult DeleteText(nsGenericDOMDataNode& aElement,
uint32_t aOffset, uint32_t aLength);
- already_AddRefed<DeleteTextTransaction>
- CreateTxnForDeleteText(nsGenericDOMDataNode& aElement,
- uint32_t aOffset, uint32_t aLength);
-
- already_AddRefed<DeleteTextTransaction>
- CreateTxnForDeleteCharacter(nsGenericDOMDataNode& aData, uint32_t aOffset,
- EDirection aDirection);
-
/**
* CreateTxnForSplitNode() creates a transaction to create a new node
* (left node) identical to an existing node (right node), and split the
* contents between the same point in both nodes.
*
* @param aStartOfRightNode The point to split. Its container will be
* the right node, i.e., become the new node's
* next sibling. And the point will be start
--- a/editor/libeditor/tests/test_bug646194.html
+++ b/editor/libeditor/tests/test_bug646194.html
@@ -1,18 +1,16 @@
<!doctype html>
<title>Mozilla Bug 646194</title>
<link rel=stylesheet href="/tests/SimpleTest/test.css">
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=646194"
target="_blank">Mozilla Bug 646194</a>
<iframe id="i" srcdoc="<div contenteditable=true id=t>test me now</div>"></iframe>
<script>
-SimpleTest.expectAssertions(1);
-
function runTest() {
var i = document.getElementById("i");
i.focus();
var win = i.contentWindow;
var doc = i.contentDocument;
var t = doc.getElementById("t");
t.focus();
// put the caret at the end