LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Feb 2007 00:10:16 +0000 (00:10 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Feb 2007 00:10:16 +0000 (00:10 +0000)
        Reviewed by john

        <rdar://problem/5012665>
        Removing indent from list moves the caret to the line below

        No new layout tests needed because the new results
        for these tests and the removed FIXMEs cover the bug fix:
        * editing/execCommand/indent-list-item.html:
        * editing/execCommand/indent-list-item-expected.checksum:
        * editing/execCommand/indent-list-item-expected.png:
        * editing/execCommand/indent-list-item-expected.txt:
        * editing/execCommand/remove-list-1.html:
        * editing/execCommand/remove-list-1-expected.checksum:
        * editing/execCommand/remove-list-1-expected.png:
        * editing/execCommand/remove-list-1-expected.txt:

WebCore:

        Reviewed by john

        <rdar://problem/5012665>
        Removing indent from list moves the caret to the line below

        Selection preservation during indent, outdent and list
        operations uses rangeFromLocationAndLength.  Ranges returned
        by rangeFromLocationAndLength were incorrect for locations
        just before the line breaks that are emitted after blocks.
        This is because TextIterator emitted bad ranges for these line
        breaks (ranges that started and ended *after* the block).
        The fix corrects the start but not the end.  This is acceptible
        because there is code in rangeFromLocationAndLength that corrects
        the ends of runs using the start of the run and VisiblePosition
        creation.

        * editing/TextIterator.cpp:
        (WebCore::TextIterator::exitNode): Emit a position *inside*
        the block, after its contents.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@19780 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/indent-list-item-expected.checksum
LayoutTests/editing/execCommand/indent-list-item-expected.png
LayoutTests/editing/execCommand/indent-list-item-expected.txt
LayoutTests/editing/execCommand/indent-list-item.html
LayoutTests/editing/execCommand/remove-list-1-expected.checksum
LayoutTests/editing/execCommand/remove-list-1-expected.png
LayoutTests/editing/execCommand/remove-list-1-expected.txt
LayoutTests/editing/execCommand/remove-list-1.html
WebCore/ChangeLog
WebCore/editing/TextIterator.cpp

index 191fab905efe8e52f6a93d5d94f441387219f5f9..e64e25e7c57ec73a042fe9f474e4b0fc9fafd611 100644 (file)
@@ -1,3 +1,21 @@
+2007-02-21  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by john
+
+        <rdar://problem/5012665>
+        Removing indent from list moves the caret to the line below
+
+        No new layout tests needed because the new results
+        for these tests and the removed FIXMEs cover the bug fix:
+        * editing/execCommand/indent-list-item.html:
+        * editing/execCommand/indent-list-item-expected.checksum:
+        * editing/execCommand/indent-list-item-expected.png:
+        * editing/execCommand/indent-list-item-expected.txt:
+        * editing/execCommand/remove-list-1.html:
+        * editing/execCommand/remove-list-1-expected.checksum:
+        * editing/execCommand/remove-list-1-expected.png:
+        * editing/execCommand/remove-list-1-expected.txt:
+
 2007-02-20  Adele Peterson  <adele@apple.com>
 
         Reviewed by Darin.
 2007-02-20  Adele Peterson  <adele@apple.com>
 
         Reviewed by Darin.
index 149d57d0780baca92ecef3a98c8fc998e8b997ec..f1b3d0577786501541a344ff0b57b8a0557fbb96 100644 (file)
@@ -1 +1 @@
-a3917529f70b6c58ebb655f2b4988073
\ No newline at end of file
+9e610294e20545bacfc2bedfb37899fa
\ No newline at end of file
index 02ed228cb3e6cc087f4217f74f16511c51ef5c47..73bd0b5adf351631fb3c9cfea6594a893a12bf1a 100644 (file)
Binary files a/LayoutTests/editing/execCommand/indent-list-item-expected.png and b/LayoutTests/editing/execCommand/indent-list-item-expected.png differ
index 3f566c97c5ceebc2933bae9d44c2f0beaca8120e..72182e563ce525bf9bf78b6d7c449269ca6b2901 100644 (file)
@@ -1,7 +1,7 @@
 EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 3 of DIV > BODY > HTML > #document
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 3 of DIV > BODY > HTML > #document
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > LI > UL > UL > DIV > BODY > HTML > #document to 1 of #text > LI > UL > UL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of LI > UL > UL > DIV > BODY > HTML > #document to 0 of LI > UL > UL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -9,16 +9,12 @@ layer at (0,0) size 800x600
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x576
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x576
-      RenderBlock {DIV} at (0,0) size 784x36
-        RenderText {#text} at (0,0) size 378x18
-          text run at (0,0) width 378: "This test uses the execCommand to Outdent the text below. "
-        RenderInline {B} at (0,0) size 775x36
-          RenderText {#text} at (378,0) size 775x36
-            text run at (378,0) width 397: "It demonstrates a bug: the caret is one position off after the"
-            text run at (0,18) width 107: "indent operatio."
-      RenderBlock (anonymous) at (0,36) size 784x18
+      RenderBlock {DIV} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 374x18
+          text run at (0,0) width 374: "This test uses the execCommand to Outdent the text below."
+      RenderBlock (anonymous) at (0,18) size 784x18
         RenderBR {BR} at (0,0) size 0x18
         RenderBR {BR} at (0,0) size 0x18
-      RenderBlock {DIV} at (0,70) size 784x54
+      RenderBlock {DIV} at (0,52) size 784x54
         RenderBlock {UL} at (0,0) size 784x54
           RenderListItem {LI} at (40,0) size 744x18
             RenderListMarker at (-17,0) size 7x18: bullet
         RenderBlock {UL} at (0,0) size 784x54
           RenderListItem {LI} at (40,0) size 744x18
             RenderListMarker at (-17,0) size 7x18: bullet
@@ -33,4 +29,4 @@ layer at (0,0) size 800x600
             RenderListMarker at (-17,0) size 7x18: bullet
             RenderText {#text} at (0,0) size 25x18
               text run at (0,0) width 25: "Baz"
             RenderListMarker at (-17,0) size 7x18: bullet
             RenderText {#text} at (0,0) size 25x18
               text run at (0,0) width 25: "Baz"
-caret: position 1 of child 0 {#text} of child 0 {LI} of child 3 {UL} of child 1 {UL} of child 4 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 0 of child 0 {#text} of child 0 {LI} of child 3 {UL} of child 1 {UL} of child 4 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index ea2a8f4253a446babee1be393d4112a1ff5a920b..8b500c6eb4391782ccb15a4a766cf1925d9584c7 100644 (file)
@@ -2,7 +2,7 @@
 if (window.layoutTestController)
      layoutTestController.dumpEditingCallbacks();
 </script>
 if (window.layoutTestController)
      layoutTestController.dumpEditingCallbacks();
 </script>
-<div id="description">This test uses the execCommand to Outdent the text below. <b>It demonstrates a bug: the caret is one position off after the indent operatio.</b></div>
+<div id="description">This test uses the execCommand to Outdent the text below.</div>
 <br>
 <div contenteditable="true">
 <ul>
 <br>
 <div contenteditable="true">
 <ul>
index 9aebb0e2638847ab3670b658f7103bf4e6cfa0f0..d049811a515c704930a7ee518e187dc3f0aa70c3 100644 (file)
@@ -1 +1 @@
-958726dee6c5c5c73089d158c46f17c5
\ No newline at end of file
+0a504896d082b314e14f07b595d94f72
\ No newline at end of file
index 0cf1c88640bbe9b0a98dc4e38d7b689a7724f66a..e3bc6f8555fe2b0e133a8b3a53ff41db3cd3d5ff 100644 (file)
Binary files a/LayoutTests/editing/execCommand/remove-list-1-expected.png and b/LayoutTests/editing/execCommand/remove-list-1-expected.png differ
index bd450a79cd4e9a4eac318370d2aa113a049c8187..f12f221c14bf05af98d1c593e7048b5d4de3e76e 100644 (file)
@@ -4,7 +4,7 @@ EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 49 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -12,15 +12,12 @@ layer at (0,0) size 800x600
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x584
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x584
-      RenderBlock {P} at (0,0) size 784x36
-        RenderText {#text} at (0,0) size 179x18
-          text run at (0,0) width 179: "This tests de-listing content. "
-        RenderInline {B} at (0,0) size 768x36
-          RenderText {#text} at (179,0) size 768x36
-            text run at (179,0) width 589: "This demonstrates a bug: the entire editable region should be selected (selections should"
-            text run at (0,18) width 324: "be preserved when performing a list command)."
-      RenderBlock {DIV} at (0,52) size 784x36
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 175x18
+          text run at (0,0) width 175: "This tests de-listing content."
+      RenderBlock {DIV} at (0,34) size 784x36
         RenderBR {BR} at (0,0) size 0x18
         RenderText {#text} at (0,18) size 322x18
           text run at (0,18) width 322: "There should be a empty paragraph above this one."
         RenderBR {BR} at (0,0) size 0x18
         RenderText {#text} at (0,18) size 322x18
           text run at (0,18) width 322: "There should be a empty paragraph above this one."
-caret: position 0 of child 0 {BR} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+selection start: position 0 of child 0 {BR} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+selection end:   position 49 of child 1 {#text} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 45b027342fc01280c21c8f9b976dcc294356e017..ee6646fd50c3866cd14b33dd7decf7bd3fc0e3b3 100644 (file)
@@ -2,7 +2,7 @@
 if (window.layoutTestController)
      layoutTestController.dumpEditingCallbacks();
 </script>
 if (window.layoutTestController)
      layoutTestController.dumpEditingCallbacks();
 </script>
-<p>This tests de-listing content. <b>This demonstrates a bug: the entire editable region should be selected (selections should be preserved when performing a list command).</b></p>
+<p>This tests de-listing content.</p>
 <div id="div" contenteditable="true"><ol><li></li><li>There should be a empty paragraph above this one.</li></ol></div>
 
 <script>
 <div id="div" contenteditable="true"><ol><li></li><li>There should be a empty paragraph above this one.</li></ol></div>
 
 <script>
index 1b551cee18c5cb99eb69dd1b0322067a2987e388..7dc26d2793e3b738ebb7860bff784080e64b5371 100644 (file)
@@ -1,3 +1,25 @@
+2007-02-21  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by john
+        
+        <rdar://problem/5012665>
+        Removing indent from list moves the caret to the line below
+
+        Selection preservation during indent, outdent and list 
+        operations uses rangeFromLocationAndLength.  Ranges returned 
+        by rangeFromLocationAndLength were incorrect for locations 
+        just before the line breaks that are emitted after blocks.  
+        This is because TextIterator emitted bad ranges for these line 
+        breaks (ranges that started and ended *after* the block).  
+        The fix corrects the start but not the end.  This is acceptible 
+        because there is code in rangeFromLocationAndLength that corrects 
+        the ends of runs using the start of the run and VisiblePosition 
+        creation.
+        
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::exitNode): Emit a position *inside* 
+        the block, after its contents.
+
 2007-02-21  Adele Peterson  <adele@apple.com>
 
         Reviewed by Darin.
 2007-02-21  Adele Peterson  <adele@apple.com>
 
         Reviewed by Darin.
index b4e671f42bacb710d0ecd44f2482ccdc23528a5e..a9eb359aa1dff86c071c6bc24844ecd683ab50a0 100644 (file)
@@ -502,23 +502,27 @@ bool TextIterator::handleNonTextNode()
 
 void TextIterator::exitNode()
 {
 
 void TextIterator::exitNode()
 {
+    // Emit with a position *inside* m_node, after m_node's contents, in 
+    // case it is a block, because the run should start where the 
+    // emitted character is positioned visually.
+    Node* baseNode = m_node->lastChild() ? m_node->lastChild() : m_node;
     if (m_lastTextNode && shouldEmitNewlineAfterNode(m_node)) {
         // use extra newline to represent margin bottom, as needed
         bool addNewline = shouldEmitExtraNewlineForNode(m_node);
         
         if (m_lastCharacter != '\n') {
     if (m_lastTextNode && shouldEmitNewlineAfterNode(m_node)) {
         // use extra newline to represent margin bottom, as needed
         bool addNewline = shouldEmitExtraNewlineForNode(m_node);
         
         if (m_lastCharacter != '\n') {
-            // insert a newline with a position following this block
-            emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+            // insert a newline with a position following this block's contents.
+            emitCharacter('\n', baseNode->parentNode(), baseNode, 1, 1);
 
             // remember whether to later add a newline for the current node
             assert(!m_needAnotherNewline);
             m_needAnotherNewline = addNewline;
         } else if (addNewline) {
 
             // remember whether to later add a newline for the current node
             assert(!m_needAnotherNewline);
             m_needAnotherNewline = addNewline;
         } else if (addNewline) {
-            // insert a newline with a position following this block
-            emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+            // insert a newline with a position following this block's contents.
+            emitCharacter('\n', baseNode->parentNode(), baseNode, 1, 1);
         }
     } else if (shouldEmitSpaceBeforeAndAfterNode(m_node))
         }
     } else if (shouldEmitSpaceBeforeAndAfterNode(m_node))
-        emitCharacter(' ', m_node->parentNode(), m_node, 1, 1);
+        emitCharacter(' ', baseNode->parentNode(), baseNode, 1, 1);
 }
 
 void TextIterator::emitCharacter(UChar c, Node *textNode, Node *offsetBaseNode, int textStartOffset, int textEndOffset)
 }
 
 void TextIterator::emitCharacter(UChar c, Node *textNode, Node *offsetBaseNode, int textStartOffset, int textEndOffset)