Bug 1480055 - part 2: Rename EditorBase::GetShouldTxnSetSelection() to EditorBase::AllowsTransactionsToChangeSelection() r?m_kato
For explaining what it does clearer, we should rename it and corresponding
member.
MozReview-Commit-ID: 6U8FgfHBbCL
--- a/editor/libeditor/CreateElementTransaction.cpp
+++ b/editor/libeditor/CreateElementTransaction.cpp
@@ -100,17 +100,17 @@ CreateElementTransaction::DoTransaction(
// Insert the new node
ErrorResult error;
InsertNewNode(error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
// Only set selection to insertion point if editor gives permission
- if (!mEditorBase->GetShouldTxnSetSelection()) {
+ if (!mEditorBase->AllowsTransactionsToChangeSelection()) {
// Do nothing - DOM range gravity will adjust selection
return NS_OK;
}
RefPtr<Selection> selection = mEditorBase->GetSelection();
NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
EditorRawDOMPoint afterNewNode(mNewNode);
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -85,28 +85,28 @@ DeleteRangeTransaction::DoTransaction()
}
// if we've successfully built this aggregate transaction, then do it.
nsresult rv = EditAggregateTransaction::DoTransaction();
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
- // only set selection to deletion point if editor gives permission
- if (mEditorBase->GetShouldTxnSetSelection()) {
- RefPtr<Selection> selection = mEditorBase->GetSelection();
- if (NS_WARN_IF(!selection)) {
- return NS_ERROR_NULL_POINTER;
- }
- rv = selection->Collapse(startRef.AsRaw());
- if (NS_WARN_IF(NS_FAILED(rv))) {
- return rv;
- }
+ if (!mEditorBase->AllowsTransactionsToChangeSelection()) {
+ return NS_OK;
}
- // else do nothing - dom range gravity will adjust selection
+
+ RefPtr<Selection> selection = mEditorBase->GetSelection();
+ if (NS_WARN_IF(!selection)) {
+ return NS_ERROR_NULL_POINTER;
+ }
+ rv = selection->Collapse(startRef.AsRaw());
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ return rv;
+ }
return NS_OK;
}
NS_IMETHODIMP
DeleteRangeTransaction::UndoTransaction()
{
return EditAggregateTransaction::UndoTransaction();
--- a/editor/libeditor/DeleteTextTransaction.cpp
+++ b/editor/libeditor/DeleteTextTransaction.cpp
@@ -132,29 +132,29 @@ DeleteTextTransaction::DoTransaction()
mCharData->DeleteData(mOffset, mLengthToDelete, err);
if (NS_WARN_IF(err.Failed())) {
return err.StealNSResult();
}
mEditorBase->RangeUpdaterRef().
SelAdjDeleteText(mCharData, mOffset, mLengthToDelete);
- // Only set selection to deletion point if editor gives permission
- if (mEditorBase->GetShouldTxnSetSelection()) {
- RefPtr<Selection> selection = mEditorBase->GetSelection();
- 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();
- }
+ if (!mEditorBase->AllowsTransactionsToChangeSelection()) {
+ return NS_OK;
}
- // Else do nothing - DOM Range gravity will adjust selection
+
+ RefPtr<Selection> selection = mEditorBase->GetSelection();
+ 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();
+ }
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
DeleteTextTransaction::UndoTransaction()
{
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -158,17 +158,17 @@ EditorBase::EditorBase()
, mModCount(0)
, mFlags(0)
, mUpdateCount(0)
, mPlaceholderBatch(0)
, mTopLevelEditSubAction(EditSubAction::eNone)
, mDirection(eNone)
, mDocDirtyState(-1)
, mSpellcheckCheckboxState(eTriUnset)
- , mShouldTxnSetSelection(true)
+ , mAllowsTransactionsToChangeSelection(true)
, mDidPreDestroy(false)
, mDidPostCreate(false)
, mDispatchInputEvent(true)
, mIsInEditSubAction(false)
, mHidingCaret(false)
, mSpellCheckerDictionaryUpdated(true)
, mIsHTMLEditorClass(false)
{
@@ -980,17 +980,17 @@ EditorBase::EndPlaceholderTransaction()
}
}
mPlaceholderBatch--;
}
NS_IMETHODIMP
EditorBase::SetShouldTxnSetSelection(bool aShould)
{
- mShouldTxnSetSelection = aShould;
+ mAllowsTransactionsToChangeSelection = aShould;
return NS_OK;
}
NS_IMETHODIMP
EditorBase::GetDocumentIsEmpty(bool* aDocumentIsEmpty)
{
return NS_ERROR_NOT_IMPLEMENTED;
}
@@ -3149,17 +3149,18 @@ EditorBase::DoSplitNode(const EditorDOMP
// Handle selection
nsCOMPtr<nsIPresShell> ps = GetPresShell();
if (ps) {
ps->FlushPendingNotifications(FlushType::Frames);
}
NS_WARNING_ASSERTION(!Destroyed(),
"The editor is destroyed during splitting a node");
- bool shouldSetSelection = GetShouldTxnSetSelection();
+ bool allowedTransactionsToChangeSelection =
+ AllowsTransactionsToChangeSelection();
RefPtr<Selection> previousSelection;
for (size_t i = 0; i < savedRanges.Length(); ++i) {
// Adjust the selection if needed.
SavedRange& range = savedRanges[i];
// If we have not seen the selection yet, clear all of its ranges.
if (range.mSelection != previousSelection) {
@@ -3167,18 +3168,18 @@ EditorBase::DoSplitNode(const EditorDOMP
if (NS_WARN_IF(aError.Failed())) {
return;
}
previousSelection = range.mSelection;
}
// XXX Looks like that we don't need to modify normal selection here
// because selection will be modified by the caller if
- // GetShouldTxnSetSelection() will return true.
- if (shouldSetSelection &&
+ // AllowsTransactionsToChangeSelection() will return true.
+ if (allowedTransactionsToChangeSelection &&
range.mSelection->Type() == SelectionType::eNormal) {
// If the editor should adjust the selection, don't bother restoring
// the ranges for the normal selection here.
continue;
}
// Split the selection into existing node and new node.
if (range.mStartContainer == aStartOfRightNode.GetContainer()) {
@@ -3307,34 +3308,35 @@ EditorBase::DoJoinNodes(nsINode* aNodeTo
}
}
}
// Delete the extra node.
ErrorResult err;
aParent->RemoveChild(*aNodeToJoin, err);
- bool shouldSetSelection = GetShouldTxnSetSelection();
+ bool allowedTransactionsToChangeSelection =
+ AllowsTransactionsToChangeSelection();
RefPtr<Selection> previousSelection;
for (size_t i = 0; i < savedRanges.Length(); ++i) {
// And adjust the selection if needed.
SavedRange& range = savedRanges[i];
// If we have not seen the selection yet, clear all of its ranges.
if (range.mSelection != previousSelection) {
ErrorResult rv;
range.mSelection->RemoveAllRanges(rv);
if (NS_WARN_IF(rv.Failed())) {
return rv.StealNSResult();
}
previousSelection = range.mSelection;
}
- if (shouldSetSelection &&
+ if (allowedTransactionsToChangeSelection &&
range.mSelection->Type() == SelectionType::eNormal) {
// If the editor should adjust the selection, don't bother restoring
// the ranges for the normal selection here.
continue;
}
// Check to see if we joined nodes where selection starts.
if (range.mStartContainer == aNodeToJoin) {
@@ -3359,20 +3361,22 @@ EditorBase::DoJoinNodes(nsINode* aNodeTo
NS_ENSURE_SUCCESS(rv, rv);
ErrorResult err;
range.mSelection->AddRange(*newRange, err);
if (NS_WARN_IF(err.Failed())) {
return err.StealNSResult();
}
}
- if (shouldSetSelection) {
+ if (allowedTransactionsToChangeSelection) {
// Editor wants us to set selection at join point.
RefPtr<Selection> selection = GetSelection();
- NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
+ if (NS_WARN_IF(!selection)) {
+ return NS_ERROR_FAILURE;
+ }
selection->Collapse(aNodeToKeep, AssertedCast<int32_t>(firstNodeLength));
}
return err.StealNSResult();
}
// static
int32_t
@@ -4149,22 +4153,16 @@ EditorBase::EndUpdateViewBatch()
if (selection) {
selection->EndBatchChanges();
}
}
return NS_OK;
}
-bool
-EditorBase::GetShouldTxnSetSelection()
-{
- return mShouldTxnSetSelection;
-}
-
TextComposition*
EditorBase::GetComposition() const
{
return mComposition;
}
bool
EditorBase::IsIMEComposing() const
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -1541,17 +1541,25 @@ protected: // May be called by friends.
* Used by table cell selection methods.
*/
nsresult CreateRange(nsINode* aStartContainer, int32_t aStartOffset,
nsINode* aEndContainer, int32_t aEndOffset,
nsRange** aRange);
static bool IsPreformatted(nsINode* aNode);
- bool GetShouldTxnSetSelection();
+ /**
+ * AllowsTransactionsToChangeSelection() returns true if editor allows any
+ * transactions to change Selection. Otherwise, transactions shouldn't
+ * change Selection.
+ */
+ inline bool AllowsTransactionsToChangeSelection() const
+ {
+ return mAllowsTransactionsToChangeSelection;
+ }
nsresult HandleInlineSpellCheck(EditSubAction aEditSubAction,
Selection& aSelection,
nsINode* previousSelectedNode,
uint32_t previousSelectedOffset,
nsINode* aStartContainer,
uint32_t aStartOffset,
nsINode* aEndContainer,
@@ -1942,18 +1950,19 @@ protected:
// The top level edit sub-action's direction.
EDirection mDirection;
// -1 = not initialized
int8_t mDocDirtyState;
// A Tristate value.
uint8_t mSpellcheckCheckboxState;
- // Turn off for conservative selection adjustment by transactions.
- bool mShouldTxnSetSelection;
+ // If false, transactions should not change Selection even after modifying
+ // the DOM tree.
+ bool mAllowsTransactionsToChangeSelection;
// Whether PreDestroy has been called.
bool mDidPreDestroy;
// Whether PostCreate has been called.
bool mDidPostCreate;
bool mDispatchInputEvent;
// True while the instance is handling an edit sub-action.
bool mIsInEditSubAction;
// Whether caret is hidden forcibly.
--- a/editor/libeditor/EditorUtils.h
+++ b/editor/libeditor/EditorUtils.h
@@ -570,35 +570,37 @@ protected:
* by low level transactions
*/
class MOZ_RAII AutoTransactionsConserveSelection final
{
public:
explicit AutoTransactionsConserveSelection(EditorBase* aEditorBase
MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
: mEditorBase(aEditorBase)
- , mOldState(true)
+ , mAllowedTransactionsToChangeSelection(true)
{
MOZ_GUARD_OBJECT_NOTIFIER_INIT;
if (mEditorBase) {
- mOldState = mEditorBase->GetShouldTxnSetSelection();
+ mAllowedTransactionsToChangeSelection =
+ mEditorBase->AllowsTransactionsToChangeSelection();
mEditorBase->SetShouldTxnSetSelection(false);
}
}
~AutoTransactionsConserveSelection()
{
if (mEditorBase) {
- mEditorBase->SetShouldTxnSetSelection(mOldState);
+ mEditorBase->SetShouldTxnSetSelection(
+ mAllowedTransactionsToChangeSelection);
}
}
protected:
EditorBase* mEditorBase;
- bool mOldState;
+ bool mAllowedTransactionsToChangeSelection;
MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
};
/***************************************************************************
* stack based helper class for batching reflow and paint requests.
*/
class MOZ_RAII AutoUpdateViewBatch final
{
--- a/editor/libeditor/InsertNodeTransaction.cpp
+++ b/editor/libeditor/InsertNodeTransaction.cpp
@@ -108,31 +108,33 @@ InsertNodeTransaction::DoTransaction()
mPointToInsert.GetContainer()->InsertBefore(*mContentToInsert,
mPointToInsert.GetChild(),
error);
error.WouldReportJSException();
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
- // Only set selection to insertion point if editor gives permission
- if (mEditorBase->GetShouldTxnSetSelection()) {
- RefPtr<Selection> selection = mEditorBase->GetSelection();
- if (NS_WARN_IF(!selection)) {
- return NS_ERROR_FAILURE;
- }
- // Place the selection just after the inserted element
- EditorRawDOMPoint afterInsertedNode(mContentToInsert);
- DebugOnly<bool> advanced = afterInsertedNode.AdvanceOffset();
- NS_WARNING_ASSERTION(advanced,
- "Failed to advance offset after the inserted node");
- selection->Collapse(afterInsertedNode, error);
- if (NS_WARN_IF(error.Failed())) {
- error.SuppressException();
- }
+ if (!mEditorBase->AllowsTransactionsToChangeSelection()) {
+ return NS_OK;
+ }
+
+ RefPtr<Selection> selection = mEditorBase->GetSelection();
+ if (NS_WARN_IF(!selection)) {
+ return NS_ERROR_FAILURE;
+ }
+
+ // Place the selection just after the inserted element.
+ EditorRawDOMPoint afterInsertedNode(mContentToInsert);
+ DebugOnly<bool> advanced = afterInsertedNode.AdvanceOffset();
+ NS_WARNING_ASSERTION(advanced,
+ "Failed to advance offset after the inserted node");
+ selection->Collapse(afterInsertedNode, error);
+ if (NS_WARN_IF(error.Failed())) {
+ error.SuppressException();
}
return NS_OK;
}
NS_IMETHODIMP
InsertNodeTransaction::UndoTransaction()
{
if (NS_WARN_IF(!mContentToInsert) ||
--- a/editor/libeditor/InsertTextTransaction.cpp
+++ b/editor/libeditor/InsertTextTransaction.cpp
@@ -65,28 +65,30 @@ InsertTextTransaction::DoTransaction()
ErrorResult rv;
mTextNode->InsertData(mOffset, mStringToInsert, rv);
if (NS_WARN_IF(rv.Failed())) {
return rv.StealNSResult();
}
// Only set selection to insertion point if editor gives permission
- if (mEditorBase->GetShouldTxnSetSelection()) {
+ if (mEditorBase->AllowsTransactionsToChangeSelection()) {
RefPtr<Selection> selection = mEditorBase->GetSelection();
if (NS_WARN_IF(!selection)) {
return NS_ERROR_FAILURE;
}
DebugOnly<nsresult> rv =
selection->Collapse(mTextNode, mOffset + mStringToInsert.Length());
NS_ASSERTION(NS_SUCCEEDED(rv),
"Selection could not be collapsed after insert");
} else {
// Do nothing - DOM Range gravity will adjust selection
}
+ // XXX Other transactions do not do this but its callers do.
+ // Why do this transaction do this by itself?
mEditorBase->RangeUpdaterRef().
SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
return NS_OK;
}
NS_IMETHODIMP
InsertTextTransaction::UndoTransaction()
--- a/editor/libeditor/SplitNodeTransaction.cpp
+++ b/editor/libeditor/SplitNodeTransaction.cpp
@@ -91,37 +91,42 @@ SplitNodeTransaction::DoTransaction()
mParent = mStartOfRightNode.GetContainer()->GetParentNode();
if (NS_WARN_IF(!mParent)) {
return NS_ERROR_FAILURE;
}
// Insert the new node
mEditorBase->DoSplitNode(EditorDOMPoint(mStartOfRightNode),
*mNewLeftNode, error);
+
+ if (!mEditorBase->AllowsTransactionsToChangeSelection()) {
+ if (NS_WARN_IF(error.Failed())) {
+ return error.StealNSResult();
+ }
+ return NS_OK;
+ }
+
// XXX Really odd. The result of DoSplitNode() is respected only when
// we shouldn't set selection. Otherwise, it's overridden by the
// result of Selection.Collapse().
- if (mEditorBase->GetShouldTxnSetSelection()) {
- NS_WARNING_ASSERTION(!mEditorBase->Destroyed(),
- "The editor has gone but SplitNodeTransaction keeps trying to modify "
- "Selection");
- RefPtr<Selection> selection = mEditorBase->GetSelection();
- if (NS_WARN_IF(!selection)) {
- return NS_ERROR_FAILURE;
- }
- if (NS_WARN_IF(error.Failed())) {
- // XXX This must be a bug.
- error.SuppressException();
- }
- MOZ_ASSERT(mStartOfRightNode.Offset() == mNewLeftNode->Length());
- EditorRawDOMPoint atEndOfLeftNode;
- atEndOfLeftNode.SetToEndOf(mNewLeftNode);
- selection->Collapse(atEndOfLeftNode, error);
+ NS_WARNING_ASSERTION(!mEditorBase->Destroyed(),
+ "The editor has gone but SplitNodeTransaction keeps trying to modify "
+ "Selection");
+ RefPtr<Selection> selection = mEditorBase->GetSelection();
+ if (NS_WARN_IF(!selection)) {
+ return NS_ERROR_FAILURE;
}
-
+ if (NS_WARN_IF(error.Failed())) {
+ // XXX This must be a bug.
+ error.SuppressException();
+ }
+ MOZ_ASSERT(mStartOfRightNode.Offset() == mNewLeftNode->Length());
+ EditorRawDOMPoint atEndOfLeftNode;
+ atEndOfLeftNode.SetToEndOf(mNewLeftNode);
+ selection->Collapse(atEndOfLeftNode, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
return NS_OK;
}
NS_IMETHODIMP
SplitNodeTransaction::UndoTransaction()