Bug 1447924 - part 5: Merge TextEditor::Undo()/Redo() with EditorBase::Undo()/Redo() r?m_kato
EditorBase::Undo() and EditorBase::Redo() implement only undo/redo function.
TextEditor::Undo() and TextEditor::Redo() call them with calling some
notification methods. However, this causes redundant AutoRules instance
creation and doesn't make sense to separate them under current design.
Therefore this patch merges them into TextEditor. Unfortunately, they are
XPCOM methods but we cannot implement proper overloads of non-virtual since
they are already minimized design. Fortunately, reducing only one virtual
call per undo/redo isn't so effective. So, let's keep simpler design.
Additionally, this patch makes them stop committing composition because
Chrome does not commit composition even if "undo"/"redo" is requested with
execCommand() during composition and it doesn't make sense to try to
undo/redo after committing composition since first undoable transaction
becomes the committing composition and committing composition causes
removing all transactions from redo transaction queue.
MozReview-Commit-ID: 78qlV2I9Lzk
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -861,37 +861,17 @@ EditorBase::GetTransactionManager() cons
{
nsCOMPtr<nsITransactionManager> transactionManager(mTransactionManager);
return transactionManager.forget();
}
NS_IMETHODIMP
EditorBase::Undo(uint32_t aCount)
{
- ForceCompositionEnd();
-
- if (!CanUndo()) {
- return NS_OK;
- }
-
- AutoRules beginRulesSniffing(this, EditAction::undo, nsIEditor::eNone);
-
- if (!mTransactionManager) {
- return NS_OK;
- }
-
- RefPtr<TransactionManager> transactionManager(mTransactionManager);
- for (uint32_t i = 0; i < aCount; ++i) {
- nsresult rv = transactionManager->UndoTransaction();
- NS_ENSURE_SUCCESS(rv, rv);
-
- DoAfterUndoTransaction();
- }
-
- return NS_OK;
+ return NS_ERROR_NOT_IMPLEMENTED;
}
NS_IMETHODIMP
EditorBase::CanUndo(bool* aIsEnabled,
bool* aCanUndo)
{
if (NS_WARN_IF(!aIsEnabled) || NS_WARN_IF(!aCanUndo)) {
return NS_ERROR_INVALID_ARG;
@@ -899,35 +879,17 @@ EditorBase::CanUndo(bool* aIsEnabled,
*aCanUndo = CanUndo();
*aIsEnabled = IsUndoRedoEnabled();
return NS_OK;
}
NS_IMETHODIMP
EditorBase::Redo(uint32_t aCount)
{
- if (!CanRedo()) {
- return NS_OK;
- }
-
- AutoRules beginRulesSniffing(this, EditAction::redo, nsIEditor::eNone);
-
- if (!mTransactionManager) {
- return NS_OK;
- }
-
- RefPtr<TransactionManager> transactionManager(mTransactionManager);
- for (uint32_t i = 0; i < aCount; ++i) {
- nsresult rv = transactionManager->RedoTransaction();
- NS_ENSURE_SUCCESS(rv, rv);
-
- DoAfterRedoTransaction();
- }
-
- return NS_OK;
+ return NS_ERROR_NOT_IMPLEMENTED;
}
NS_IMETHODIMP
EditorBase::CanRedo(bool* aIsEnabled, bool* aCanRedo)
{
if (NS_WARN_IF(!aIsEnabled) || NS_WARN_IF(!aCanRedo)) {
return NS_ERROR_INVALID_ARG;
}
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -1129,63 +1129,113 @@ TextEditor::SetNewlineHandling(int32_t a
mNewlineHandling = aNewlineHandling;
return NS_OK;
}
NS_IMETHODIMP
TextEditor::Undo(uint32_t aCount)
{
- // Protect the edit rules object from dying
+ // If we don't have transaction in the undo stack, we shouldn't notify
+ // anybody of trying to undo since it's not useful notification but we
+ // need to pay some runtime cost.
+ if (!CanUndo()) {
+ return NS_OK;
+ }
+
+ // If there is composition, we shouldn't allow to undo with committing
+ // composition since Chrome doesn't allow it and it doesn't make sense
+ // because committing composition causes one transaction and Undo(1)
+ // undoes the committing composition.
+ if (GetComposition()) {
+ return NS_OK;
+ }
+
+ // Protect the edit rules object from dying.
RefPtr<TextEditRules> rules(mRules);
AutoUpdateViewBatch beginViewBatching(this);
- CommitComposition();
+ NotifyEditorObservers(eNotifyEditorObserversOfBefore);
+ if (NS_WARN_IF(!CanUndo()) || NS_WARN_IF(Destroyed())) {
+ return NS_ERROR_FAILURE;
+ }
- NotifyEditorObservers(eNotifyEditorObserversOfBefore);
-
- AutoRules beginRulesSniffing(this, EditAction::undo, nsIEditor::eNone);
+ nsresult rv;
+ {
+ AutoRules beginRulesSniffing(this, EditAction::undo, nsIEditor::eNone);
- RulesInfo ruleInfo(EditAction::undo);
- RefPtr<Selection> selection = GetSelection();
- bool cancel, handled;
- nsresult rv = rules->WillDoAction(selection, &ruleInfo, &cancel, &handled);
-
- if (!cancel && NS_SUCCEEDED(rv)) {
- rv = EditorBase::Undo(aCount);
- rv = rules->DidDoAction(selection, &ruleInfo, rv);
+ RulesInfo ruleInfo(EditAction::undo);
+ RefPtr<Selection> selection = GetSelection();
+ bool cancel, handled;
+ rv = rules->WillDoAction(selection, &ruleInfo, &cancel, &handled);
+ if (!cancel && NS_SUCCEEDED(rv)) {
+ RefPtr<TransactionManager> transactionManager(mTransactionManager);
+ for (uint32_t i = 0; i < aCount; ++i) {
+ rv = transactionManager->Undo();
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ break;
+ }
+ DoAfterUndoTransaction();
+ }
+ rv = rules->DidDoAction(selection, &ruleInfo, rv);
+ }
}
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
return rv;
}
NS_IMETHODIMP
TextEditor::Redo(uint32_t aCount)
{
- // Protect the edit rules object from dying
+ // If we don't have transaction in the redo stack, we shouldn't notify
+ // anybody of trying to redo since it's not useful notification but we
+ // need to pay some runtime cost.
+ if (!CanRedo()) {
+ return NS_OK;
+ }
+
+ // If there is composition, we shouldn't allow to redo with committing
+ // composition since Chrome doesn't allow it and it doesn't make sense
+ // because committing composition causes removing all transactions from
+ // the redo queue. So, it becomes impossible to redo anything.
+ if (GetComposition()) {
+ return NS_OK;
+ }
+
+ // Protect the edit rules object from dying.
RefPtr<TextEditRules> rules(mRules);
AutoUpdateViewBatch beginViewBatching(this);
- CommitComposition();
+ NotifyEditorObservers(eNotifyEditorObserversOfBefore);
+ if (NS_WARN_IF(!CanRedo()) || NS_WARN_IF(Destroyed())) {
+ return NS_ERROR_FAILURE;
+ }
- NotifyEditorObservers(eNotifyEditorObserversOfBefore);
-
- AutoRules beginRulesSniffing(this, EditAction::redo, nsIEditor::eNone);
+ nsresult rv;
+ {
+ AutoRules beginRulesSniffing(this, EditAction::redo, nsIEditor::eNone);
- RulesInfo ruleInfo(EditAction::redo);
- RefPtr<Selection> selection = GetSelection();
- bool cancel, handled;
- nsresult rv = rules->WillDoAction(selection, &ruleInfo, &cancel, &handled);
-
- if (!cancel && NS_SUCCEEDED(rv)) {
- rv = EditorBase::Redo(aCount);
- rv = rules->DidDoAction(selection, &ruleInfo, rv);
+ RulesInfo ruleInfo(EditAction::redo);
+ RefPtr<Selection> selection = GetSelection();
+ bool cancel, handled;
+ rv = rules->WillDoAction(selection, &ruleInfo, &cancel, &handled);
+ if (!cancel && NS_SUCCEEDED(rv)) {
+ RefPtr<TransactionManager> transactionManager(mTransactionManager);
+ for (uint32_t i = 0; i < aCount; ++i) {
+ nsresult rv = transactionManager->Redo();
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ break;
+ }
+ DoAfterRedoTransaction();
+ }
+ rv = rules->DidDoAction(selection, &ruleInfo, rv);
+ }
}
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
return rv;
}
bool
TextEditor::CanCutOrCopy(PasswordFieldAllowed aPasswordFieldAllowed)
--- a/editor/libeditor/TextEditor.h
+++ b/editor/libeditor/TextEditor.h
@@ -81,18 +81,20 @@ public:
nsresult DocumentIsEmpty(bool* aIsEmpty);
NS_IMETHOD GetDocumentIsEmpty(bool* aDocumentIsEmpty) override;
NS_IMETHOD DeleteSelection(EDirection aAction,
EStripWrappers aStripWrappers) override;
NS_IMETHOD SetDocumentCharacterSet(const nsACString& characterSet) override;
- NS_IMETHOD Undo(uint32_t aCount) override;
- NS_IMETHOD Redo(uint32_t aCount) override;
+ // If there are some good name to create non-virtual Undo()/Redo() methods,
+ // we should create them and those methods should just run them.
+ NS_IMETHOD Undo(uint32_t aCount) final;
+ NS_IMETHOD Redo(uint32_t aCount) final;
NS_IMETHOD Cut() override;
NS_IMETHOD CanCut(bool* aCanCut) override;
NS_IMETHOD Copy() override;
NS_IMETHOD CanCopy(bool* aCanCopy) override;
NS_IMETHOD CanDelete(bool* aCanDelete) override;
NS_IMETHOD Paste(int32_t aSelectionType) override;
NS_IMETHOD CanPaste(int32_t aSelectionType, bool* aCanPaste) override;
--- a/editor/txmgr/TransactionManager.cpp
+++ b/editor/txmgr/TransactionManager.cpp
@@ -84,63 +84,77 @@ TransactionManager::DoTransaction(nsITra
// XXX The result of EndTransaction() or DidDoNotify() if EndTransaction()
// succeeded.
return rv;
}
NS_IMETHODIMP
TransactionManager::UndoTransaction()
{
- // It is illegal to call UndoTransaction() while the transaction manager is
- // executing a transaction's DoTransaction() method! If this happens,
- // the UndoTransaction() request is ignored, and we return NS_ERROR_FAILURE.
+ return Undo();
+}
+
+nsresult
+TransactionManager::Undo()
+{
+ // It's possible to be called Undo() again while the transaction manager is
+ // executing a transaction's DoTransaction() method. If this happens,
+ // the Undo() request is ignored, and we return NS_ERROR_FAILURE. This
+ // may occur if a mutation event listener calls document.execCommand("undo").
if (!mDoStack.IsEmpty()) {
return NS_ERROR_FAILURE;
}
// Peek at the top of the undo stack. Don't remove the transaction
// until it has successfully completed.
RefPtr<TransactionItem> transactionItem = mUndoStack.Peek();
if (!transactionItem) {
// Bail if there's nothing on the stack.
return NS_OK;
}
- nsCOMPtr<nsITransaction> t = transactionItem->GetTransaction();
+ nsCOMPtr<nsITransaction> transaction = transactionItem->GetTransaction();
bool doInterrupt = false;
- nsresult rv = WillUndoNotify(t, &doInterrupt);
+ nsresult rv = WillUndoNotify(transaction, &doInterrupt);
if (NS_FAILED(rv)) {
return rv;
}
if (doInterrupt) {
return NS_OK;
}
rv = transactionItem->UndoTransaction(this);
if (NS_SUCCEEDED(rv)) {
transactionItem = mUndoStack.Pop();
mRedoStack.Push(transactionItem.forget());
}
- nsresult rv2 = DidUndoNotify(t, rv);
+ nsresult rv2 = DidUndoNotify(transaction, rv);
if (NS_SUCCEEDED(rv)) {
rv = rv2;
}
// XXX The result of UndoTransaction() or DidUndoNotify() if UndoTransaction()
// succeeded.
return rv;
}
NS_IMETHODIMP
TransactionManager::RedoTransaction()
{
- // It is illegal to call RedoTransaction() while the transaction manager is
- // executing a transaction's DoTransaction() method! If this happens,
- // the RedoTransaction() request is ignored, and we return NS_ERROR_FAILURE.
+ return Redo();
+}
+
+nsresult
+TransactionManager::Redo()
+{
+ // It's possible to be called Redo() again while the transaction manager is
+ // executing a transaction's DoTransaction() method. If this happens,
+ // the Redo() request is ignored, and we return NS_ERROR_FAILURE. This
+ // may occur if a mutation event listener calls document.execCommand("redo").
if (!mDoStack.IsEmpty()) {
return NS_ERROR_FAILURE;
}
// Peek at the top of the redo stack. Don't remove the transaction
// until it has successfully completed.
RefPtr<TransactionItem> transactionItem = mRedoStack.Peek();
if (!transactionItem) {
--- a/editor/txmgr/TransactionManager.h
+++ b/editor/txmgr/TransactionManager.h
@@ -31,16 +31,19 @@ public:
NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(TransactionManager,
nsITransactionManager)
NS_DECL_NSITRANSACTIONMANAGER
already_AddRefed<nsITransaction> PeekUndoStack();
already_AddRefed<nsITransaction> PeekRedoStack();
+ nsresult Undo();
+ nsresult Redo();
+
size_t NumberOfUndoItems() const
{
return mUndoStack.GetSize();
}
size_t NumberOfRedoItems() const
{
return mRedoStack.GetSize();
}