Created attachment 227041 [details] Test Case 1.Fetch attached test case. 2.type some text 3.select all (ctrl+A) 4.click ordered list and then unordered list Actual behavior : Selection disappears. Expected behavior : Selection should retains.
Created attachment 227155 [details] Patch
Comment on attachment 227155 [details] Patch Attachment 227155 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5332482580807680 New failing tests: editing/execCommand/insert-list-nested-with-orphaned.html
Created attachment 227158 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 227155 [details] Patch Attachment 227155 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5361014048555008 New failing tests: editing/execCommand/insert-list-nested-with-orphaned.html
Created attachment 227172 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
In the current fix we are placing the cursor at the end of applied list item,so failed case is because of cursor poistion, but actual issue is that "This tests hang when listifying nested lists with an orphaned list child in between", actual test case works fine Please review this patch.
Comment on attachment 227155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227155&action=review > Source/WebCore/editing/InsertListCommand.cpp:246 > - setEndingSelection(VisiblePosition(firstPositionInNode(newList.get()))); > + setEndingSelection(VisibleSelection(firstPositionInNode(newList.get()), lastPositionInNode(newList.get()), endingSelection().isDirectional())); Why select the whole list when the list type is changed? That seems almost as bad as the old rule (put the selection at the start of the first list element). What is the desired selection change in a case like this? Before we implement it we need to clearly state what we want? Do we want to preserve the existing selection? Or something else? I think that when making a new list we want to put the insertion point in the list, ready to type into that list. > LayoutTests/editing/execCommand/selectall-ol-ul-expected.txt:12 > +After Applying Orderedlist: > +| <ol> > +| <li> > +| "<#selection-anchor>orderlist text to display<#selection-focus>" > +| <br> > + > +After Applying UnOrderedList: > +| <ul> > +| <li> > +| "<#selection-anchor>orderlist text to display<#selection-focus>" This test makes it look like the command preserves the selection. But what it actually does is select the entire contents of the list. I don’t think that’s right. The test needs to show what happens when the contents of the list isn’t all selected. For example, what if just the word "text" was selected? What if the insertion point was after the word "display"?