Bug 1447924 - part 6: Implement EnableUndoRedo(), DisableUndoRedo() and ClearUndoRedo() in EditorBase and TransactionManager r?m_kato
nsIEditor::EnableUndo() and nsITransactionManager::Clear(),
nsITransactionManager::SetMaxTransactionCount() are called a lot but they are
virtual and some of or all of them are called once. There should be each
non-virtual method to do what each root caller wants. Therefore, this patch
adds EditorBase::EnableUndoRedo(), EditorBase::DisableUndoRedo(),
EditorBase::ClearUndoRedo(), TransactionManager::EnableUndoRedo(),
TransactionManager::DisableUndoRedo() and TransactionManager::ClearUndoRedo().
Note that this patch makes TransactionManager won't clear mUndoStack nor
mRedoStack if mDoStack is not empty. This is checked only by
TransactionManager::SetMaxTransactionCount() but according to the comment,
TransactionManager::Clear(), TransactionManager::UndoStack() and
TransactionManager::RedoStack() should check it too.
MozReview-Commit-ID: 6qBZOQNwdhw
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -168,22 +168,32 @@ public:
MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
: mTextEditor(aTextEditor)
, mPreviousEnabled(true)
{
MOZ_GUARD_OBJECT_NOTIFIER_INIT;
MOZ_ASSERT(mTextEditor);
mPreviousEnabled = mTextEditor->IsUndoRedoEnabled();
- mTextEditor->EnableUndo(false);
+ DebugOnly<bool> disabledUndoRedo = mTextEditor->DisableUndoRedo();
+ NS_WARNING_ASSERTION(disabledUndoRedo,
+ "Failed to disable undo/redo transactions");
}
~AutoDisableUndo()
{
- mTextEditor->EnableUndo(mPreviousEnabled);
+ if (mPreviousEnabled) {
+ DebugOnly<bool> enabledUndoRedo = mTextEditor->EnableUndoRedo();
+ NS_WARNING_ASSERTION(enabledUndoRedo,
+ "Failed to enable undo/redo transactions");
+ } else {
+ DebugOnly<bool> disabledUndoRedo = mTextEditor->DisableUndoRedo();
+ NS_WARNING_ASSERTION(disabledUndoRedo,
+ "Failed to disable undo/redo transactions");
+ }
}
private:
TextEditor* mTextEditor;
bool mPreviousEnabled;
MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
};
@@ -1500,32 +1510,30 @@ nsTextEditorState::PrepareEditor(const n
bool success = SetValue(defaultValue, eSetValue_Internal);
NS_ENSURE_TRUE(success, NS_ERROR_OUT_OF_MEMORY);
// Now restore the original editor flags.
rv = newTextEditor->SetFlags(editorFlags);
NS_ENSURE_SUCCESS(rv, rv);
}
- nsCOMPtr<nsITransactionManager> transactionManager =
- newTextEditor->GetTransactionManager();
- if (NS_WARN_IF(!transactionManager)) {
- return NS_ERROR_FAILURE;
- }
-
- transactionManager->SetMaxTransactionCount(
- nsITextControlElement::DEFAULT_UNDO_CAP);
-
if (IsPasswordTextControl()) {
- // Disable undo for password textfields. Note that we want to do this at
- // the very end of InitEditor, so the calls to EnableUndo when setting the
- // default value don't screw us up.
- // Since changing the control type does a reframe, we don't have to worry
- // about dynamic type changes here.
- newTextEditor->EnableUndo(false);
+ // Disable undo for <input type="password">. Note that we want to do this
+ // at the very end of InitEditor(), so the calls to EnableUndoRedo() when
+ // setting the default value don't screw us up. Since changing the
+ // control type does a reframe, we don't have to worry about dynamic type
+ // changes here.
+ DebugOnly<bool> disabledUndoRedo = newTextEditor->DisableUndoRedo();
+ NS_WARNING_ASSERTION(disabledUndoRedo,
+ "Failed to disable undo/redo transaction");
+ } else {
+ DebugOnly<bool> enabledUndoRedo =
+ newTextEditor->EnableUndoRedo(nsITextControlElement::DEFAULT_UNDO_CAP);
+ NS_WARNING_ASSERTION(enabledUndoRedo,
+ "Failed to enable undo/redo transaction");
}
if (!mEditorInitialized) {
newTextEditor->PostCreate();
mEverInited = true;
mEditorInitialized = true;
}
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -521,17 +521,19 @@ EditorBase::PreDestroy(bool aDestroyingF
mTextServicesDocument = nullptr;
mTextInputListener = nullptr;
mSpellcheckCheckboxState = eTriUnset;
mRootElement = nullptr;
// Transaction may grab this instance. Therefore, they should be released
// here for stopping the circular reference with this instance.
if (mTransactionManager) {
- mTransactionManager->Clear();
+ DebugOnly<bool> disabledUndoRedo = DisableUndoRedo();
+ NS_WARNING_ASSERTION(disabledUndoRedo,
+ "Failed to disable undo/redo transactions");
mTransactionManager = nullptr;
}
mDidPreDestroy = true;
return NS_OK;
}
NS_IMETHODIMP
@@ -806,27 +808,27 @@ EditorBase::DoTransaction(Selection* aSe
}
return NS_OK;
}
NS_IMETHODIMP
EditorBase::EnableUndo(bool aEnable)
{
+ // XXX Should we return NS_ERROR_FAILURE if EdnableUndoRedo() or
+ // DisableUndoRedo() returns false?
if (aEnable) {
- if (!mTransactionManager) {
- mTransactionManager = new TransactionManager();
- }
- mTransactionManager->SetMaxTransactionCount(-1);
- } else if (mTransactionManager) {
- // disable the transaction manager if it is enabled
- mTransactionManager->Clear();
- mTransactionManager->SetMaxTransactionCount(0);
- }
-
+ DebugOnly<bool> enabledUndoRedo = EnableUndoRedo();
+ NS_WARNING_ASSERTION(enabledUndoRedo,
+ "Failed to enable undo/redo transactions");
+ return NS_OK;
+ }
+ DebugOnly<bool> disabledUndoRedo = DisableUndoRedo();
+ NS_WARNING_ASSERTION(disabledUndoRedo,
+ "Failed to disable undo/redo transactions");
return NS_OK;
}
NS_IMETHODIMP
EditorBase::GetNumberOfUndoItems(int32_t* aNumItems)
{
*aNumItems = static_cast<int32_t>(NumberOfUndoItems());
MOZ_ASSERT(*aNumItems >= 0);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -1127,16 +1127,44 @@ public:
return IsUndoRedoEnabled() && NumberOfUndoItems() > 0;
}
bool CanRedo() const
{
return IsUndoRedoEnabled() && NumberOfRedoItems() > 0;
}
/**
+ * Enables or disables undo/redo feature. Returns true if it succeeded,
+ * otherwise, e.g., we're undoing or redoing, returns false.
+ */
+ bool EnableUndoRedo(int32_t aMaxTransactionCount = -1)
+ {
+ if (!mTransactionManager) {
+ mTransactionManager = new TransactionManager();
+ }
+ return mTransactionManager->EnableUndoRedo(aMaxTransactionCount);
+ }
+ bool DisableUndoRedo()
+ {
+ if (!mTransactionManager) {
+ return true;
+ }
+ // XXX Even we clear the transaction manager, IsUndoRedoEnabled() keep
+ // returning true...
+ return mTransactionManager->DisableUndoRedo();
+ }
+ bool ClearUndoRedo()
+ {
+ if (!mTransactionManager) {
+ return true;
+ }
+ return mTransactionManager->ClearUndoRedo();
+ }
+
+ /**
* From html rules code - migration in progress.
*/
static nsAtom* GetTag(nsIDOMNode* aNode);
bool NodesSameType(nsIDOMNode* aNode1, nsIDOMNode* aNode2);
virtual bool AreNodesSameType(nsIContent* aNode1, nsIContent* aNode2);
static bool IsTextNode(nsIDOMNode* aNode);
--- a/editor/libeditor/EditorCommands.cpp
+++ b/editor/libeditor/EditorCommands.cpp
@@ -190,18 +190,20 @@ ClearUndoCommand::DoCommand(const char*
nsISupports* aCommandRefCon)
{
nsCOMPtr<nsIEditor> editor = do_QueryInterface(aCommandRefCon);
if (!editor) {
return NS_ERROR_FAILURE;
}
TextEditor* textEditor = editor->AsTextEditor();
MOZ_ASSERT(textEditor);
- textEditor->EnableUndo(false); // Turning off undo clears undo/redo stacks.
- textEditor->EnableUndo(true); // This re-enables undo/redo.
+ // XXX Should we return NS_ERROR_FAILURE if ClearUndoRedo() returns false?
+ DebugOnly<bool> clearedUndoRedo = textEditor->ClearUndoRedo();
+ NS_WARNING_ASSERTION(clearedUndoRedo,
+ "Failed to clear undo/redo transactions");
return NS_OK;
}
NS_IMETHODIMP
ClearUndoCommand::DoCommandParams(const char* aCommandName,
nsICommandParams* aParams,
nsISupports* aCommandRefCon)
{
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -207,18 +207,18 @@ TextEditor::EndEditorInit()
}
nsresult rv = InitRules();
if (NS_FAILED(rv)) {
return rv;
}
// Throw away the old transaction manager if this is not the first time that
// we're initializing the editor.
- EnableUndo(false);
- EnableUndo(true);
+ ClearUndoRedo();
+ EnableUndoRedo();
return NS_OK;
}
NS_IMETHODIMP
TextEditor::SetDocumentCharacterSet(const nsACString& characterSet)
{
nsresult rv = EditorBase::SetDocumentCharacterSet(characterSet);
NS_ENSURE_SUCCESS(rv, rv);
--- a/editor/txmgr/TransactionManager.cpp
+++ b/editor/txmgr/TransactionManager.cpp
@@ -186,21 +186,17 @@ TransactionManager::Redo()
// XXX The result of RedoTransaction() or DidRedoNotify() if RedoTransaction()
// succeeded.
return rv;
}
NS_IMETHODIMP
TransactionManager::Clear()
{
- nsresult rv = ClearRedoStack();
- if (NS_FAILED(rv)) {
- return rv;
- }
- return ClearUndoStack();
+ return ClearUndoRedo() ? NS_OK : NS_ERROR_FAILURE;
}
NS_IMETHODIMP
TransactionManager::BeginBatch(nsISupports* aData)
{
// We can batch independent transactions together by simply pushing
// a dummy transaction item on the do stack. This dummy transaction item
// will be popped off the do stack, and then pushed on the undo stack
@@ -290,65 +286,83 @@ TransactionManager::GetMaxTransactionCou
NS_ENSURE_TRUE(aMaxCount, NS_ERROR_NULL_POINTER);
*aMaxCount = mMaxTransactionCount;
return NS_OK;
}
NS_IMETHODIMP
TransactionManager::SetMaxTransactionCount(int32_t aMaxCount)
{
- // It is illegal to call SetMaxTransactionCount() while the transaction
- // manager is executing a transaction's DoTransaction() method because
- // the undo and redo stacks might get pruned! If this happens, the
- // SetMaxTransactionCount() request is ignored, and we return
- // NS_ERROR_FAILURE.
- if (!mDoStack.IsEmpty()) {
- return NS_ERROR_FAILURE;
+ return EnableUndoRedo(aMaxCount) ? NS_OK : NS_ERROR_FAILURE;
+}
+
+bool
+TransactionManager::EnableUndoRedo(int32_t aMaxTransactionCount)
+{
+ // It is illegal to call EnableUndoRedo() while the transaction manager is
+ // executing a transaction's DoTransaction() method because the undo and redo
+ // stacks might get pruned. If this happens, the EnableUndoRedo() request is
+ // ignored, and we return false.
+ if (NS_WARN_IF(!mDoStack.IsEmpty())) {
+ return false;
+ }
+
+ // If aMaxTransactionCount is 0, it means to disable undo/redo.
+ if (!aMaxTransactionCount) {
+ mUndoStack.Clear();
+ mRedoStack.Clear();
+ mMaxTransactionCount = 0;
+ return true;
}
- // If aMaxCount is less than zero, the user wants unlimited
- // levels of undo! No need to prune the undo or redo stacks!
- if (aMaxCount < 0) {
+ // If aMaxTransactionCount is less than zero, the user wants unlimited
+ // levels of undo! No need to prune the undo or redo stacks.
+ if (aMaxTransactionCount < 0) {
mMaxTransactionCount = -1;
- return NS_OK;
+ return true;
}
- // If aMaxCount is greater than the number of transactions that currently
- // exist on the undo and redo stack, there is no need to prune the
- // undo or redo stacks!
- int32_t numUndoItems = mUndoStack.GetSize();
- int32_t numRedoItems = mRedoStack.GetSize();
- int32_t total = numUndoItems + numRedoItems;
- if (aMaxCount > total) {
- mMaxTransactionCount = aMaxCount;
- return NS_OK;
+ // If new max transaction count is greater than or equal to current max
+ // transaction count, we don't need to remove any transactions.
+ if (mMaxTransactionCount >= 0 &&
+ mMaxTransactionCount <= aMaxTransactionCount) {
+ mMaxTransactionCount = aMaxTransactionCount;
+ return true;
+ }
+
+ // If aMaxTransactionCount is greater than the number of transactions that
+ // currently exist on the undo and redo stack, there is no need to prune the
+ // undo or redo stacks.
+ size_t numUndoItems = NumberOfUndoItems();
+ size_t numRedoItems = NumberOfRedoItems();
+ size_t total = numUndoItems + numRedoItems;
+ size_t newMaxTransactionCount = static_cast<size_t>(aMaxTransactionCount);
+ if (newMaxTransactionCount > total) {
+ mMaxTransactionCount = aMaxTransactionCount;
+ return true;
}
// Try getting rid of some transactions on the undo stack! Start at
// the bottom of the stack and pop towards the top.
- while (numUndoItems > 0 && (numRedoItems + numUndoItems) > aMaxCount) {
+ for (; numUndoItems && (numRedoItems + numUndoItems) > newMaxTransactionCount;
+ numUndoItems--) {
RefPtr<TransactionItem> transactionItem = mUndoStack.PopBottom();
- if (!transactionItem) {
- return NS_ERROR_FAILURE;
- }
- --numUndoItems;
+ MOZ_ASSERT(transactionItem);
}
// If necessary, get rid of some transactions on the redo stack! Start at
// the bottom of the stack and pop towards the top.
- while (numRedoItems > 0 && (numRedoItems + numUndoItems) > aMaxCount) {
+ for (; numRedoItems && (numRedoItems + numUndoItems) > newMaxTransactionCount;
+ numRedoItems--) {
RefPtr<TransactionItem> transactionItem = mRedoStack.PopBottom();
- if (!transactionItem) {
- return NS_ERROR_FAILURE;
- }
- --numRedoItems;
+ MOZ_ASSERT(transactionItem);
}
- mMaxTransactionCount = aMaxCount;
- return NS_OK;
+ mMaxTransactionCount = aMaxTransactionCount;
+ return true;
}
NS_IMETHODIMP
TransactionManager::PeekUndoStack(nsITransaction** aTransaction)
{
MOZ_ASSERT(aTransaction);
*aTransaction = PeekUndoStack().take();
return NS_OK;
@@ -430,23 +444,29 @@ TransactionManager::RemoveListener(nsITr
{
NS_ENSURE_TRUE(aListener, NS_ERROR_NULL_POINTER);
return mListeners.RemoveObject(aListener) ? NS_OK : NS_ERROR_FAILURE;
}
NS_IMETHODIMP
TransactionManager::ClearUndoStack()
{
+ if (NS_WARN_IF(!mDoStack.IsEmpty())) {
+ return NS_ERROR_FAILURE;
+ }
mUndoStack.Clear();
return NS_OK;
}
NS_IMETHODIMP
TransactionManager::ClearRedoStack()
{
+ if (NS_WARN_IF(!mDoStack.IsEmpty())) {
+ return NS_ERROR_FAILURE;
+ }
mRedoStack.Clear();
return NS_OK;
}
nsresult
TransactionManager::WillDoNotify(nsITransaction* aTransaction,
bool* aInterrupt)
{
@@ -695,20 +715,17 @@ TransactionManager::EndTransaction(bool
// be added to the transaction at the top of the do stack.
RefPtr<TransactionItem> topTransactionItem = mDoStack.Peek();
if (topTransactionItem) {
// XXX: What do we do if this fails?
return topTransactionItem->AddChild(transactionItem);
}
// The transaction succeeded, so clear the redo stack.
- rv = ClearRedoStack();
- if (NS_FAILED(rv)) {
- // XXX: What do we do if this fails?
- }
+ mRedoStack.Clear();
// Check if we can coalesce this transaction with the one at the top
// of the undo stack.
topTransactionItem = mUndoStack.Peek();
if (transaction && topTransactionItem) {
bool didMerge = false;
nsCOMPtr<nsITransaction> topTransaction =
topTransactionItem->GetTransaction();
--- a/editor/txmgr/TransactionManager.h
+++ b/editor/txmgr/TransactionManager.h
@@ -43,16 +43,31 @@ public:
{
return mUndoStack.GetSize();
}
size_t NumberOfRedoItems() const
{
return mRedoStack.GetSize();
}
+ bool EnableUndoRedo(int32_t aMaxTransactionCount = -1);
+ bool DisableUndoRedo()
+ {
+ return EnableUndoRedo(0);
+ }
+ bool ClearUndoRedo()
+ {
+ if (NS_WARN_IF(!mDoStack.IsEmpty())) {
+ return false;
+ }
+ mUndoStack.Clear();
+ mRedoStack.Clear();
+ return true;
+ }
+
nsresult WillDoNotify(nsITransaction* aTransaction, bool* aInterrupt);
nsresult DidDoNotify(nsITransaction* aTransaction, nsresult aExecuteResult);
nsresult WillUndoNotify(nsITransaction* aTransaction, bool* aInterrupt);
nsresult DidUndoNotify(nsITransaction* aTransaction, nsresult aUndoResult);
nsresult WillRedoNotify(nsITransaction* aTransaction, bool *aInterrupt);
nsresult DidRedoNotify(nsITransaction* aTransaction, nsresult aRedoResult);
nsresult WillBeginBatchNotify(bool* aInterrupt);
nsresult DidBeginBatchNotify(nsresult aResult);