Bug 1328023 - Part 1. Don't use RangeUpdater except to composition transaction. r=masayuki, a=jcristau
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Mon, 30 Jan 2017 17:25:43 +0900
changeset 375781 e235e56f062a8bb2bfec3ea17fa098ecd422b476
parent 375780 0e1d4619f4c6e18f9add95dce45a6969f7310a00
child 375782 46497fa848f7b678b5d1bbfd28ed8d1828443b9b
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki, jcristau
bugs1328023, 1310912
milestone53.0a2
Bug 1328023 - Part 1. Don't use RangeUpdater except to composition transaction. r=masayuki, a=jcristau 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();