Bug 1480055 - part 2: Rename EditorBase::GetShouldTxnSetSelection() to EditorBase::AllowsTransactionsToChangeSelection() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 01 Aug 2018 20:53:57 +0900
changeset 487712 a3ee99eced65c73a8c3ba0b5d1ee5a47235aa362
parent 487711 6b0f8b1b33c0401543df1055bd1e0fd2060a1ebe
child 487713 702671b11b7c6fe20898f2ef464d2088a14c60f1
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1480055
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
editor/libeditor/CreateElementTransaction.cpp
editor/libeditor/DeleteRangeTransaction.cpp
editor/libeditor/DeleteTextTransaction.cpp
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/EditorUtils.h
editor/libeditor/InsertNodeTransaction.cpp
editor/libeditor/InsertTextTransaction.cpp
editor/libeditor/SplitNodeTransaction.cpp
--- 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()