Fix issue with selection being lost when InsertListCommand changes the list type.

Source/core/editing/InsertListCommand.cpp

Index: Source/core/editing/InsertListCommand.cpp
diff --git a/Source/core/editing/InsertListCommand.cpp b/Source/core/editing/InsertListCommand.cpp
index 0d07431ba83045d411b6c113631cd32c7576f5a7..d0e6dfa4ccd6b708f984b78a194d742a9c20d25b 100644
--- a/Source/core/editing/InsertListCommand.cpp
+++ b/Source/core/editing/InsertListCommand.cpp
@@ -129,18 +129,25 @@ void InsertListCommand::doApply()
 
     const QualifiedName& listTag = (m_type == OrderedList) ? olTag : ulTag;
     if (endingSelection().isRange()) {
+        bool forceListCreation = false;
         VisibleSelection selection = selectionForParagraphIteration(endingSelection());
         ASSERT(selection.isRange());
         VisiblePosition startOfSelection = selection.visibleStart();
         VisiblePosition endOfSelection = selection.visibleEnd();
         VisiblePosition startOfLastParagraph = startOfParagraph(endOfSelection, CanSkipOverEditingBoundary);
 
+        RefPtrWillBeRawPtr<Range> currentSelection = endingSelection().firstRange();
+        RefPtrWillBeRawPtr<ContainerNode> scopeForStartOfSelection = nullptr;
+        RefPtrWillBeRawPtr<ContainerNode> scopeForEndOfSelection = nullptr;
+        // FIXME: This is an inefficient way to keep selection alive because indexForVisiblePosition walks from

[tkent, 2014/06/27 05:04:57]

nit: recommend to wrap comments in 80-columns.

[arpitabahuguna, 2014/06/27 06:15:30]

Done.

+        // the beginning of the document to the endOfSelection everytime this code is executed.
+        // But not using index is hard because there are so many ways we can los eselection inside doApplyForSingleParagraph.
+        int indexForStartOfSelection = indexForVisiblePosition(startOfSelection, scopeForStartOfSelection);
+        int indexForEndOfSelection = indexForVisiblePosition(endOfSelection, scopeForEndOfSelection);
+
         if (startOfParagraph(startOfSelection, CanSkipOverEditingBoundary) != startOfLastParagraph) {
-            RefPtrWillBeRawPtr<ContainerNode> scope = nullptr;
-            int indexForEndOfSelection = indexForVisiblePosition(endOfSelection, scope);
-            bool forceCreateList = !selectionHasListOfType(selection, listTag);
+            forceListCreation = !selectionHasListOfType(selection, listTag);
 
-            RefPtrWillBeRawPtr<Range> currentSelection = endingSelection().firstRange();
             VisiblePosition startOfCurrentParagraph = startOfSelection;
             while (startOfCurrentParagraph.isNotNull() && !inSameParagraph(startOfCurrentParagraph, startOfLastParagraph, CanCrossEditingBoundary)) {
                 // doApply() may operate on and remove the last paragraph of the selection from the document
@@ -154,12 +161,9 @@ void InsertListCommand::doApply()
 
                 // Save and restore endOfSelection and startOfLastParagraph when necessary
                 // since moveParagraph and movePragraphWithClones can remove nodes.
-                // FIXME: This is an inefficient way to keep selection alive because indexForVisiblePosition walks from
-                // the beginning of the document to the endOfSelection everytime this code is executed.
-                // But not using index is hard because there are so many ways we can lose selection inside doApplyForSingleParagraph.
-                doApplyForSingleParagraph(forceCreateList, listTag, *currentSelection);
+                doApplyForSingleParagraph(forceListCreation, listTag, *currentSelection);
                 if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
-                    endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scope.get());
+                    endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scopeForEndOfSelection.get());
                     // If endOfSelection is null, then some contents have been deleted from the document.
                     // This should never happen and if it did, exit early immediately because we've lost the loop invariant.
                     ASSERT(endOfSelection.isNotNull());
@@ -168,26 +172,24 @@ void InsertListCommand::doApply()
                     startOfLastParagraph = startOfParagraph(endOfSelection, CanSkipOverEditingBoundary);
                 }
 
-                // Fetch the start of the selection after moving the first paragraph,
-                // because moving the paragraph will invalidate the original start.
-                // We'll use the new start to restore the original selection after
-                // we modified all selected paragraphs.
-                if (startOfCurrentParagraph == startOfSelection)
-                    startOfSelection = endingSelection().visibleStart();
-
                 startOfCurrentParagraph = startOfNextParagraph(endingSelection().visibleStart());
             }
             setEndingSelection(endOfSelection);
-            doApplyForSingleParagraph(forceCreateList, listTag, *currentSelection);
-            // Fetch the end of the selection, for the reason mentioned above.
-            if (endOfSelection.isNull() || endOfSelection.isOrphan()) {
-                endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scope.get());
-                if (endOfSelection.isNull())
-                    return;
-            }
-            setEndingSelection(VisibleSelection(startOfSelection, endOfSelection, endingSelection().isDirectional()));
-            return;
         }
+        doApplyForSingleParagraph(forceListCreation, listTag, *currentSelection);
+        // Fetch the end of the selection, for the reason mentioned above.
+        if (endOfSelection.isNull() || endOfSelection.isOrphan()) {
+            endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scopeForEndOfSelection.get());
+            if (endOfSelection.isNull())
+                return;
+        }
+        if (startOfSelection.isNull() || startOfSelection.isOrphan()) {
+            startOfSelection = visiblePositionForIndex(indexForStartOfSelection, scopeForStartOfSelection.get());
+            if (startOfSelection.isNull())
+                return;
+        }
+        setEndingSelection(VisibleSelection(startOfSelection, endOfSelection, endingSelection().isDirectional()));
+        return;
     }
 
     ASSERT(endingSelection().firstRange());