Bug 1100966 - Remember all ranges for all selections when joining nodes in the editor transactions; r=roc
authorEhsan Akhgari <ehsan@mozilla.com>
Sat, 07 Mar 2015 17:14:07 -0500
changeset 233469 64ae62dab70bfb53325feb3fb8caec7cf549cd2d
parent 233468 91abcf24159bb6837f598e39e1f9da190696f83e
child 233470 c26740d56f2331bea4b0576b47731ebb0ee0af16
push id11757
push usercbook@mozilla.com
push dateFri, 13 Mar 2015 13:11:51 +0000
treeherderfx-team@31fb00e91bc7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1100966
milestone39.0a1
Bug 1100966 - Remember all ranges for all selections when joining nodes in the editor transactions; r=roc This patch fixes some symptoms, the most common of which misspelling ranges disappearing when performing some editor operations.
editor/libeditor/nsEditor.cpp
editor/libeditor/nsEditor.h
editor/libeditor/tests/chrome.ini
editor/libeditor/tests/test_bug1100966.html
--- a/editor/libeditor/nsEditor.cpp
+++ b/editor/libeditor/nsEditor.cpp
@@ -613,33 +613,38 @@ nsEditor::GetSelectionController(nsISele
 NS_IMETHODIMP
 nsEditor::DeleteSelection(EDirection aAction, EStripWrappers aStripWrappers)
 {
   MOZ_ASSERT(aStripWrappers == eStrip || aStripWrappers == eNoStrip);
   return DeleteSelectionImpl(aAction, aStripWrappers);
 }
 
 
-
 NS_IMETHODIMP
-nsEditor::GetSelection(nsISelection **aSelection)
+nsEditor::GetSelection(nsISelection** aSelection)
+{
+  return GetSelection(nsISelectionController::SELECTION_NORMAL, aSelection);
+}
+
+nsresult
+nsEditor::GetSelection(int16_t aSelectionType, nsISelection** aSelection)
 {
   NS_ENSURE_TRUE(aSelection, NS_ERROR_NULL_POINTER);
   *aSelection = nullptr;
   nsCOMPtr<nsISelectionController> selcon;
   GetSelectionController(getter_AddRefs(selcon));
   NS_ENSURE_TRUE(selcon, NS_ERROR_NOT_INITIALIZED);
-  return selcon->GetSelection(nsISelectionController::SELECTION_NORMAL, aSelection);  // does an addref
+  return selcon->GetSelection(aSelectionType, aSelection);  // does an addref
 }
 
 Selection*
-nsEditor::GetSelection()
+nsEditor::GetSelection(int16_t aSelectionType)
 {
   nsCOMPtr<nsISelection> sel;
-  nsresult res = GetSelection(getter_AddRefs(sel));
+  nsresult res = GetSelection(aSelectionType, getter_AddRefs(sel));
   NS_ENSURE_SUCCESS(res, nullptr);
 
   return static_cast<Selection*>(sel.get());
 }
 
 NS_IMETHODIMP
 nsEditor::DoTransaction(nsITransaction* aTxn)
 {
@@ -2681,135 +2686,172 @@ nsEditor::SplitNodeImpl(nsIContent& aExi
     }
     selection->Collapse(selStartNode, selStartOffset);
     selection->Extend(selEndNode, selEndOffset);
   }
 
   return NS_OK;
 }
 
+struct SavedRange {
+  nsRefPtr<Selection> mSelection;
+  nsCOMPtr<nsINode> mStartNode;
+  nsCOMPtr<nsINode> mEndNode;
+  int32_t mStartOffset;
+  int32_t mEndOffset;
+};
+
 nsresult
 nsEditor::JoinNodesImpl(nsINode* aNodeToKeep,
                         nsINode* aNodeToJoin,
                         nsINode* aParent)
 {
   MOZ_ASSERT(aNodeToKeep);
   MOZ_ASSERT(aNodeToJoin);
   MOZ_ASSERT(aParent);
 
-  nsRefPtr<Selection> selection = GetSelection();
-  NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
-
-  // remember some selection points
-  nsCOMPtr<nsINode> selStartNode;
-  int32_t selStartOffset;
-  nsresult result = GetStartNodeAndOffset(selection, getter_AddRefs(selStartNode), &selStartOffset);
-  if (NS_FAILED(result)) {
-    selStartNode = nullptr;
-  }
-
-  nsCOMPtr<nsINode> selEndNode;
-  int32_t selEndOffset;
-  result = GetEndNodeAndOffset(selection, getter_AddRefs(selEndNode), &selEndOffset);
-  // Joe or Kin should comment here on why the following line is not a copy/paste error
-  if (NS_FAILED(result)) {
-    selStartNode = nullptr;
-  }
-
   uint32_t firstNodeLength = aNodeToJoin->Length();
 
   int32_t joinOffset;
   GetNodeLocation(aNodeToJoin, &joinOffset);
   int32_t keepOffset;
   nsINode* parent = GetNodeLocation(aNodeToKeep, &keepOffset);
 
-  // if selection endpoint is between the nodes, remember it as being
-  // in the one that is going away instead.  This simplifies later selection
-  // adjustment logic at end of this method.
-  if (selStartNode) {
-    if (selStartNode == parent &&
-        joinOffset < selStartOffset && selStartOffset <= keepOffset) {
-      selStartNode = aNodeToJoin;
-      selStartOffset = firstNodeLength;
+  // Remember all selection points.
+  nsAutoTArray<SavedRange, 10> savedRanges;
+  for (size_t i = 0; i < nsISelectionController::NUM_SELECTIONTYPES - 1; ++i) {
+    SelectionType type(1 << i);
+    SavedRange range;
+    range.mSelection = GetSelection(type);
+    if (type == nsISelectionController::SELECTION_NORMAL) {
+      NS_ENSURE_TRUE(range.mSelection, NS_ERROR_NULL_POINTER);
+    } else if (!range.mSelection) {
+      // For non-normal selections, skip over the non-existing ones.
+      continue;
     }
-    if (selEndNode == parent &&
-        joinOffset < selEndOffset && selEndOffset <= keepOffset) {
-      selEndNode = aNodeToJoin;
-      selEndOffset = firstNodeLength;
+
+    for (uint32_t j = 0; j < range.mSelection->RangeCount(); ++j) {
+      nsRefPtr<nsRange> r = range.mSelection->GetRangeAt(j);
+      if (!r->IsPositioned()) {
+        continue;
+      }
+      range.mStartNode = r->GetStartParent();
+      range.mStartOffset = r->StartOffset();
+      range.mEndNode = r->GetEndParent();
+      range.mEndOffset = r->EndOffset();
+
+      // If selection endpoint is between the nodes, remember it as being
+      // in the one that is going away instead.  This simplifies later selection
+      // adjustment logic at end of this method.
+      if (range.mStartNode) {
+        if (range.mStartNode == parent &&
+            joinOffset < range.mStartOffset &&
+            range.mStartOffset <= keepOffset) {
+          range.mStartNode = aNodeToJoin;
+          range.mStartOffset = firstNodeLength;
+        }
+        if (range.mEndNode == parent &&
+            joinOffset < range.mEndOffset &&
+            range.mEndOffset <= keepOffset) {
+          range.mEndNode = aNodeToJoin;
+          range.mEndOffset = firstNodeLength;
+        }
+      }
+
+      savedRanges.AppendElement(range);
     }
   }
 
-  // ok, ready to do join now.
-  // if it's a text node, just shuffle around some text
+  // OK, ready to do join now.
+  // If it's a text node, just shuffle around some text.
   nsCOMPtr<nsIDOMCharacterData> keepNodeAsText( do_QueryInterface(aNodeToKeep) );
   nsCOMPtr<nsIDOMCharacterData> joinNodeAsText( do_QueryInterface(aNodeToJoin) );
   if (keepNodeAsText && joinNodeAsText) {
     nsAutoString rightText;
     nsAutoString leftText;
     keepNodeAsText->GetData(rightText);
     joinNodeAsText->GetData(leftText);
     leftText += rightText;
     keepNodeAsText->SetData(leftText);
   } else {
-    // otherwise it's an interior node, so shuffle around the children
+    // Otherwise it's an interior node, so shuffle around the children.
     nsCOMPtr<nsINodeList> childNodes = aNodeToJoin->ChildNodes();
     MOZ_ASSERT(childNodes);
 
-    // remember the first child in aNodeToKeep, we'll insert all the children of aNodeToJoin in front of it
-    // GetFirstChild returns nullptr firstNode if aNodeToKeep has no children, that's ok.
+    // Remember the first child in aNodeToKeep, we'll insert all the children of aNodeToJoin in front of it
+    // GetFirstChild returns nullptr firstNode if aNodeToKeep has no children, that's OK.
     nsCOMPtr<nsIContent> firstNode = aNodeToKeep->GetFirstChild();
 
-    // have to go through the list backwards to keep deletes from interfering with iteration
+    // Have to go through the list backwards to keep deletes from interfering with iteration.
     for (uint32_t i = childNodes->Length(); i > 0; --i) {
       nsCOMPtr<nsIContent> childNode = childNodes->Item(i - 1);
       if (childNode) {
         // prepend children of aNodeToJoin
         ErrorResult err;
         aNodeToKeep->InsertBefore(*childNode, firstNode, err);
         NS_ENSURE_SUCCESS(err.ErrorCode(), err.ErrorCode());
         firstNode = childNode.forget();
       }
     }
   }
 
-  // delete the extra node
+  // Delete the extra node.
   ErrorResult err;
   aParent->RemoveChild(*aNodeToJoin, err);
 
-  if (GetShouldTxnSetSelection()) {
-    // editor wants us to set selection at join point
-    selection->Collapse(aNodeToKeep, AssertedCast<int32_t>(firstNodeLength));
-  } else if (selStartNode) {
-    // and adjust the selection if needed
-    // HACK: this is overly simplified - multi-range selections need more work than this
-    bool bNeedToAdjust = false;
-
-    // check to see if we joined nodes where selection starts
-    if (selStartNode == aNodeToJoin) {
-      bNeedToAdjust = true;
-      selStartNode = aNodeToKeep;
-    } else if (selStartNode == aNodeToKeep) {
-      bNeedToAdjust = true;
-      selStartOffset += firstNodeLength;
+  bool shouldSetSelection = GetShouldTxnSetSelection();
+
+  nsRefPtr<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) {
+      nsresult rv = range.mSelection->RemoveAllRanges();
+      NS_ENSURE_SUCCESS(rv, rv);
+      previousSelection = range.mSelection;
+    }
+
+    if (shouldSetSelection &&
+        range.mSelection->Type() ==
+          nsISelectionController::SELECTION_NORMAL) {
+      // 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 ends
-    if (selEndNode == aNodeToJoin) {
-      bNeedToAdjust = true;
-      selEndNode = aNodeToKeep;
-    } else if (selEndNode == aNodeToKeep) {
-      bNeedToAdjust = true;
-      selEndOffset += firstNodeLength;
+    // Check to see if we joined nodes where selection starts.
+    if (range.mStartNode == aNodeToJoin) {
+      range.mStartNode = aNodeToKeep;
+    } else if (range.mStartNode == aNodeToKeep) {
+      range.mStartOffset += firstNodeLength;
+    }
+
+    // Check to see if we joined nodes where selection ends.
+    if (range.mEndNode == aNodeToJoin) {
+      range.mEndNode = aNodeToKeep;
+    } else if (range.mEndNode == aNodeToKeep) {
+      range.mEndOffset += firstNodeLength;
     }
 
-    // adjust selection if needed
-    if (bNeedToAdjust) {
-      selection->Collapse(selStartNode, selStartOffset);
-      selection->Extend(selEndNode, selEndOffset);
-    }
+    nsRefPtr<nsRange> newRange;
+    nsresult rv = nsRange::CreateRange(range.mStartNode, range.mStartOffset,
+                                       range.mEndNode, range.mEndOffset,
+                                       getter_AddRefs(newRange));
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = range.mSelection->AddRange(newRange);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  if (shouldSetSelection) {
+    // Editor wants us to set selection at join point.
+    nsRefPtr<Selection> selection = GetSelection();
+    NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
+    selection->Collapse(aNodeToKeep, AssertedCast<int32_t>(firstNodeLength));
   }
 
   return err.ErrorCode();
 }
 
 
 int32_t
 nsEditor::GetChildOffset(nsIDOMNode* aChild, nsIDOMNode* aParent)
--- a/editor/libeditor/nsEditor.h
+++ b/editor/libeditor/nsEditor.h
@@ -13,16 +13,17 @@
 #include "nsCOMPtr.h"                   // for already_AddRefed, nsCOMPtr
 #include "nsCycleCollectionParticipant.h"
 #include "nsGkAtoms.h"
 #include "nsIEditor.h"                  // for nsIEditor::EDirection, etc
 #include "nsIEditorIMESupport.h"        // for NS_DECL_NSIEDITORIMESUPPORT, etc
 #include "nsIObserver.h"                // for NS_DECL_NSIOBSERVER, etc
 #include "nsIPhonetic.h"                // for NS_DECL_NSIPHONETIC, etc
 #include "nsIPlaintextEditor.h"         // for nsIPlaintextEditor, etc
+#include "nsISelectionController.h"     // for nsISelectionController constants
 #include "nsISupportsImpl.h"            // for nsEditor::Release, etc
 #include "nsIWeakReferenceUtils.h"      // for nsWeakPtr
 #include "nsLiteralString.h"            // for NS_LITERAL_STRING
 #include "nsSelectionState.h"           // for nsRangeUpdater, etc
 #include "nsString.h"                   // for nsCString
 #include "nsWeakReference.h"            // for nsSupportsWeakReference
 #include "nscore.h"                     // for nsresult, nsAString, etc
 
@@ -416,16 +417,18 @@ protected:
   }
 
   /**
    * EnsureComposition() should be called by composition event handlers.  This
    * tries to get the composition for the event and set it to mComposition.
    */
   void EnsureComposition(mozilla::WidgetGUIEvent* aEvent);
 
+  nsresult GetSelection(int16_t aSelectionType, nsISelection** aSelection);
+
 public:
 
   /** All editor operations which alter the doc should be prefaced
    *  with a call to StartOperation, naming the action and direction */
   NS_IMETHOD StartOperation(EditAction opID,
                             nsIEditor::EDirection aDirection);
 
   /** All editor operations which alter the doc should be followed
@@ -613,17 +616,18 @@ public:
                                       nsIDOMNode** outEndNode,
                                       int32_t* outEndOffset);
   static nsresult GetEndNodeAndOffset(mozilla::dom::Selection* aSelection,
                                       nsINode** aEndNode,
                                       int32_t* aEndOffset);
 #if DEBUG_JOE
   static void DumpNode(nsIDOMNode *aNode, int32_t indent=0);
 #endif
-  mozilla::dom::Selection* GetSelection();
+  mozilla::dom::Selection* GetSelection(int16_t aSelectionType =
+      nsISelectionController::SELECTION_NORMAL);
 
   // Helpers to add a node to the selection. 
   // Used by table cell selection methods
   nsresult CreateRange(nsIDOMNode *aStartParent, int32_t aStartOffset,
                        nsIDOMNode *aEndParent, int32_t aEndOffset,
                        nsRange** aRange);
 
   // Creates a range with just the supplied node and appends that to the selection
--- a/editor/libeditor/tests/chrome.ini
+++ b/editor/libeditor/tests/chrome.ini
@@ -15,16 +15,17 @@ skip-if = buildapp == 'mulet'
 [test_bug607584.xul]
 [test_bug616590.xul]
 [test_bug635636.html]
 [test_bug636465.xul]
 [test_bug646194.xul]
 [test_bug780908.xul]
 [test_bug830600.html]
 [test_bug1053048.html]
+[test_bug1100966.html]
 [test_bug1102906.html]
 [test_bug1101392.html]
 [test_composition_event_created_in_chrome.html]
 [test_contenteditable_text_input_handling.html]
 [test_dragdrop.html]
 skip-if = buildapp == 'mulet'
 [test_htmleditor_keyevent_handling.html]
 [test_selection_move_commands.xul]
new file mode 100644
--- /dev/null
+++ b/editor/libeditor/tests/test_bug1100966.html
@@ -0,0 +1,64 @@
+<!DOCTYPE>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1100966
+-->
+<head>
+  <title>Test for Bug 1100966</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+  <script type="text/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
+</head>
+<body>
+<div id="display">
+</div>
+<div id="content" contenteditable>
+=====<br>
+correct<br>
+fivee sixx<br>
+====
+</div>
+<pre id="test">
+</pre>
+
+<script class="testbody" type="application/javascript">
+
+/** Test for Bug 1100966 **/
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(function() {
+  var div = document.getElementById("content");
+  div.focus();
+  synthesizeMouseAtCenter(div, {});
+
+  synthesizeKey(" ", {});
+  setTimeout(function() {
+    synthesizeKey("a", {});
+    setTimeout(function() {
+      synthesizeKey("VK_BACK_SPACE", {});
+
+      var sel = getSpellCheckSelection();
+      is(sel.rangeCount, 2, "We should have two misspelled words");
+      is(sel.getRangeAt(0), "fivee", "Correct misspelled word");
+      is(sel.getRangeAt(1), "sixx", "Correct misspelled word");
+
+      SimpleTest.finish();
+    },0);
+  },0);
+
+});
+
+function getSpellCheckSelection() {
+  var Ci = Components.interfaces;
+  var editingSession = window.QueryInterface(Ci.nsIInterfaceRequestor)
+                             .getInterface(Ci.nsIWebNavigation)
+                             .QueryInterface(Ci.nsIInterfaceRequestor)
+                             .getInterface(Ci.nsIEditingSession);
+  var editor = editingSession.getEditorForWindow(window);
+  var selcon = editor.selectionController;
+  return selcon.getSelection(selcon.SELECTION_SPELLCHECK);
+}
+
+</script>
+</body>
+
+</html>