Bug 1328023 - Part 1. Don't use RangeUpdater except to composition transaction. r?masayuki draft
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Mon, 30 Jan 2017 17:25:43 +0900
changeset 467907 59cf67e6713557c7659b13fe154d77d1122fca56
parent 467834 71224049c0b52ab190564d3ea0eab089a159a4cf
child 467908 4ea638a45dcbcaca47c29c80290823faf39f3636
push id43303
push userbmo:m_kato@ga2.so-net.ne.jp
push dateMon, 30 Jan 2017 10:36:16 +0000
reviewersmasayuki
bugs1328023, 1310912
milestone54.0a1
Bug 1328023 - Part 1. Don't use RangeUpdater except to composition transaction. r?masayuki Part 3 fix of bug 1310912 is incorrect for not composition transaction. PlaceholderTransation is for saving and restoring current selection for undo. So we shouldn't use range updater to normal transaction. Composition transaction can modify multiple nodes and it merges text node for ime into single text node. So if current selection is into IME text node, it might be failed to restore selection by UndoTransaction. So we need update selection by range updater to work UndoTransaction. Also, CompositionTransaction::UndoTransaction will set selection after committed text. So at finally, selection will set correct position that composition transaction wants. MozReview-Commit-ID: 1NcH32YoKPQ
editor/libeditor/EditorBase.cpp
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -922,17 +922,24 @@ EditorBase::BeginPlaceHolderTransaction(
     // time to turn on the batch
     BeginUpdateViewBatch();
     mPlaceHolderTxn = nullptr;
     mPlaceHolderName = aName;
     RefPtr<Selection> selection = GetSelection();
     if (selection) {
       mSelState = new SelectionState();
       mSelState->SaveSelection(selection);
-      mRangeUpdater.RegisterSelectionState(*mSelState);
+      // Composition transaction can modify multiple nodes and it merges text
+      // node for ime into single text node.
+      // So if current selection is into IME text node, it might be failed
+      // to restore selection by UndoTransaction.
+      // So we need update selection by range updater.
+      if (mPlaceHolderName == nsGkAtoms::IMETxnName) {
+        mRangeUpdater.RegisterSelectionState(*mSelState);
+      }
     }
   }
   mPlaceHolderBatch++;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -974,17 +981,19 @@ EditorBase::EndPlaceHolderTransaction()
     // cached for frame offset are Not available now
     if (selection) {
       selection->SetCanCacheFrameOffset(false);
     }
 
     if (mSelState) {
       // we saved the selection state, but never got to hand it to placeholder
       // (else we ould have nulled out this pointer), so destroy it to prevent leaks.
-      mRangeUpdater.DropSelectionState(*mSelState);
+      if (mPlaceHolderName == nsGkAtoms::IMETxnName) {
+        mRangeUpdater.DropSelectionState(*mSelState);
+      }
       delete mSelState;
       mSelState = nullptr;
     }
     // We might have never made a placeholder if no action took place.
     if (mPlaceHolderTxn) {
       nsCOMPtr<nsIAbsorbingTransaction> plcTxn = do_QueryReferent(mPlaceHolderTxn);
       if (plcTxn) {
         plcTxn->EndPlaceHolderBatch();