Reviewed by John and Ken.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 13 Mar 2005 21:35:32 +0000 (21:35 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 13 Mar 2005 21:35:32 +0000 (21:35 +0000)
        - fixed <rdar://problem/4044347> REGRESSION (Mail): Control-K in particular message moves insertion point to previous line

        Tweaked the deleting code, and added three new deleting layout tests to confirm the new code works.

        * khtml/editing/htmlediting.cpp:
        (khtml::DeleteSelectionCommand::handleSpecialCaseBRDelete): Removed special case with comment that said it was
        for the case where a "selection contains only a BR right after a block ended". This code was being triggered in
        more cases than just that one, and in all the cases I tested, the general delete code works fine.
        (khtml::DeleteSelectionCommand::handleGeneralDelete): Changed the code that decides whether to delete an entire
        block to understand the case where the end block is outside the start block, but contains the start block.
        In that case, we want to delete the entire block. Not deleting the block was causing us to delete just the <br>,
        and not the enclosing <div> in the case in the bug.

        * layout-tests/editing/deleting/delete-line-015-expected.txt: Added.
        * layout-tests/editing/deleting/delete-line-015.html: Added.
        * layout-tests/editing/deleting/delete-line-016-expected.txt: Added.
        * layout-tests/editing/deleting/delete-line-016.html: Added.
        * layout-tests/editing/deleting/delete-line-017-expected.txt: Added.
        * layout-tests/editing/deleting/delete-line-017.html: Added.
        * layout-tests/editing/style/smoosh-styles-002-expected.txt: Updated to improved results. With the code change, the deletion
        now deletes more than it used to. The old results had an empty text node and <h1> element that were both 0-sized, and now
        we delete both of those.

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

LayoutTests/editing/deleting/delete-line-015-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-line-015.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-line-016-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-line-016.html [new file with mode: 0644]
LayoutTests/editing/deleting/delete-line-017-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-line-017.html [new file with mode: 0644]
LayoutTests/editing/style/smoosh-styles-002-expected.txt
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp

diff --git a/LayoutTests/editing/deleting/delete-line-015-expected.txt b/LayoutTests/editing/deleting/delete-line-015-expected.txt
new file mode 100644 (file)
index 0000000..baae299
--- /dev/null
@@ -0,0 +1,20 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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
+      RenderBlock {P} at (0,0) size 784x54
+        RenderText {TEXT} at (0,0) size 778x54
+          text run at (0,0) width 764: "This tests deletion of an empty line starting inside a <div> element, but ending outside that element (just before a <br>). If"
+          text run at (0,18) width 778: "the deletion is successful, the result should have two lines, and the insertion point should be on the second line, at the end of"
+          text run at (0,36) width 106: "the editable area."
+      RenderBlock {DIV} at (0,70) size 784x36
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {TEXT} at (0,0) size 7x18
+            text run at (0,0) width 7: "a"
+        RenderBlock (anonymous) at (0,18) size 784x18
+          RenderBR {BR} at (0,0) size 0x18
+selection is CARET:
+start:      position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+upstream:   position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+downstream: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
diff --git a/LayoutTests/editing/deleting/delete-line-015.html b/LayoutTests/editing/deleting/delete-line-015.html
new file mode 100644 (file)
index 0000000..4938548
--- /dev/null
@@ -0,0 +1,28 @@
+<html> 
+
+<head>
+<script src="../editing.js"></script>
+<script>
+function editingTest() {
+    extendSelectionForwardByLineCommand();    
+    deleteCommand(); 
+}
+</script>
+</head> 
+
+<body>
+
+<p>This tests deletion of an empty line starting inside a &lt;div&gt; element,
+but ending outside that element (just before a &lt;br&gt;). If the deletion is
+successful, the result should have two lines, and the insertion point
+should be on the second line, at the end of the editable area.</p>
+
+<div contenteditable>
+<div>a</div><div id="test"><br></div><br>
+</div>
+
+</body>
+
+<script>runEditingTest()</script>
+
+</html>
diff --git a/LayoutTests/editing/deleting/delete-line-016-expected.txt b/LayoutTests/editing/deleting/delete-line-016-expected.txt
new file mode 100644 (file)
index 0000000..24ab7ac
--- /dev/null
@@ -0,0 +1,20 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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
+      RenderBlock {P} at (0,0) size 784x54
+        RenderText {TEXT} at (0,0) size 767x54
+          text run at (0,0) width 767: "This tests deletion of an empty line starting inside a <div> element, but ending outside that element (just before a <br>). In"
+          text run at (0,18) width 766: "this case, the <div> has no content inside it, but a min-height style prevents it from collapsing. If the deletion is successful,"
+          text run at (0,36) width 725: "the result should have two lines, and the insertion point should be on the second line, at the end of the editable area."
+      RenderBlock {DIV} at (0,70) size 784x36
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {TEXT} at (0,0) size 7x18
+            text run at (0,0) width 7: "a"
+        RenderBlock (anonymous) at (0,18) size 784x18
+          RenderBR {BR} at (0,0) size 0x18
+selection is CARET:
+start:      position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+upstream:   position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+downstream: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
diff --git a/LayoutTests/editing/deleting/delete-line-016.html b/LayoutTests/editing/deleting/delete-line-016.html
new file mode 100644 (file)
index 0000000..5fe8dfb
--- /dev/null
@@ -0,0 +1,29 @@
+<html> 
+
+<head>
+<script src="../editing.js"></script>
+<script>
+function editingTest() {
+    extendSelectionForwardByLineCommand();    
+    deleteCommand(); 
+}
+</script>
+</head> 
+
+<body>
+
+<p>This tests deletion of an empty line starting inside a &lt;div&gt; element,
+but ending outside that element (just before a &lt;br&gt;). In this case, the
+&lt;div&gt; has no content inside it, but a min-height style prevents it from
+collapsing. If the deletion is successful, the result should have two lines,
+and the insertion point should be on the second line, at the end of the editable area.</p>
+
+<div contenteditable>
+<div>a</div><div id="test" style="min-height:50px"></div><br>
+</div>
+
+</body>
+
+<script>runEditingTest()</script>
+
+</html>
diff --git a/LayoutTests/editing/deleting/delete-line-017-expected.txt b/LayoutTests/editing/deleting/delete-line-017-expected.txt
new file mode 100644 (file)
index 0000000..164c8bc
--- /dev/null
@@ -0,0 +1,21 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {TEXT} at (0,0) size 781x36
+          text run at (0,0) width 781: "This tests deletion of an empty line which is a <br> element just after a <div> element. If the deletion is successful, the result"
+          text run at (0,18) width 695: "should have two lines, and the insertion point should be at the start of the second line, just before the letter \"b\"."
+      RenderBlock {DIV} at (0,52) size 784x36
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {TEXT} at (0,0) size 7x18
+            text run at (0,0) width 7: "a"
+        RenderBlock (anonymous) at (0,18) size 784x0
+        RenderBlock {DIV} at (0,18) size 784x18
+          RenderText {TEXT} at (0,0) size 8x18
+            text run at (0,0) width 8: "b"
+selection is CARET:
+start:      position 0 of child 1 {TEXT} of child 2 {DIV} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+upstream:   position 0 of child 2 {DIV} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+downstream: position 0 of child 1 {TEXT} of child 2 {DIV} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
diff --git a/LayoutTests/editing/deleting/delete-line-017.html b/LayoutTests/editing/deleting/delete-line-017.html
new file mode 100644 (file)
index 0000000..449858e
--- /dev/null
@@ -0,0 +1,28 @@
+<html> 
+
+<head>
+<script src="../editing.js"></script>
+<script>
+function editingTest() {
+    moveSelectionForwardByLineCommand();    
+    extendSelectionForwardByLineCommand();    
+    deleteCommand(); 
+}
+</script>
+</head> 
+
+<body>
+
+<p>This tests deletion of an empty line which is a &lt;br&gt; element just after
+a &lt;div&gt; element. If the deletion is successful, the result should have two lines,
+and the insertion point should be at the start of the second line, just before the letter "b".</p>
+
+<div contenteditable>
+<div id="test">a</div><br><div>b</div>
+</div>
+
+</body>
+
+<script>runEditingTest()</script>
+
+</html>
index 586a995ee9f13eb4316320e7c357619db68a676a..21db69c3d5ca850a0b3b4902bac400294faa1cb4 100644 (file)
@@ -2,7 +2,7 @@ layer at (0,0) size 800x600
   RenderCanvas at (0,0) size 800x600
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
-    RenderBody {BODY} at (8,8) size 784x571
+    RenderBody {BODY} at (8,8) size 784x584
       RenderBlock {DIV} at (0,0) size 784x165 [border: (2px solid #0000FF)]
         RenderBlock {DIV} at (14,14) size 756x56
           RenderText {TEXT} at (0,0) size 67x28
@@ -37,8 +37,6 @@ layer at (0,0) size 800x600
                     text run at (31,2) width 46: "cde"
             RenderText {TEXT} at (77,8) size 24x28
               text run at (77,8) width 24: "fg"
-          RenderText {TEXT} at (0,0) size 0x0
-        RenderBlock {H1} at (0,62) size 784x0
 selection is CARET:
 start:      position 3 of child 1 {TEXT} of child 1 {B} of child 1 {SPAN} of child 2 {FONT} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
 upstream:   position 3 of child 1 {TEXT} of child 1 {B} of child 1 {SPAN} of child 2 {FONT} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
index 019b7eea6e3e326a668673096cef30bec1cc4d33..9a5de7bd1d5dc2e5b5fc9e21184777462490e7a4 100644 (file)
@@ -1,3 +1,30 @@
+2005-03-13  Darin Adler  <darin@apple.com>
+
+        Reviewed by John and Ken.
+
+        - fixed <rdar://problem/4044347> REGRESSION (Mail): Control-K in particular message moves insertion point to previous line
+
+        Tweaked the deleting code, and added three new deleting layout tests to confirm the new code works.
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::DeleteSelectionCommand::handleSpecialCaseBRDelete): Removed special case with comment that said it was
+        for the case where a "selection contains only a BR right after a block ended". This code was being triggered in
+        more cases than just that one, and in all the cases I tested, the general delete code works fine.
+        (khtml::DeleteSelectionCommand::handleGeneralDelete): Changed the code that decides whether to delete an entire
+        block to understand the case where the end block is outside the start block, but contains the start block.
+        In that case, we want to delete the entire block. Not deleting the block was causing us to delete just the <br>,
+        and not the enclosing <div> in the case in the bug.
+
+        * layout-tests/editing/deleting/delete-line-015-expected.txt: Added.
+        * layout-tests/editing/deleting/delete-line-015.html: Added.
+        * layout-tests/editing/deleting/delete-line-016-expected.txt: Added.
+        * layout-tests/editing/deleting/delete-line-016.html: Added.
+        * layout-tests/editing/deleting/delete-line-017-expected.txt: Added.
+        * layout-tests/editing/deleting/delete-line-017.html: Added.
+        * layout-tests/editing/style/smoosh-styles-002-expected.txt: Updated to improved results. With the code change, the deletion
+        now deletes more than it used to. The old results had an empty text node and <h1> element that were both 0-sized, and now
+        we delete both of those.
+
 2005-03-13  Darin Adler  <darin@apple.com>
 
         - fixed <rdar://problem/4049172> REGRESSION (403-405): Gmail: text box in "Invite a friend" section overlaps other sections
         * khtml/rendering/render_flow.cpp:
         (RenderFlow::paintLines):
 
-<<<<<<< ChangeLog
 2005-03-01  Chris Blumenberg  <cblu@apple.com>
 
        Fixed: <rdar://problem/4030669> smart delete does not delete spaces from pasted content
index 4286d03b0a4500d3cb28dd4b0694495a0c8c1878..602ba9ccfd47f09995fa581419cf6c325df053c5 100644 (file)
@@ -2484,19 +2484,6 @@ bool DeleteSelectionCommand::handleSpecialCaseBRDelete()
         return true;
     }
 
-    // Check for special-case where the selection contains only a BR right after a block ended.
-    bool downstreamEndIsBR = m_downstreamEnd.node()->id() == ID_BR;
-    Position upstreamFromBR = m_downstreamEnd.upstream();
-    Position downstreamFromStart = m_downstreamStart.downstream();
-    bool startIsBRAfterBlock = downstreamEndIsBR && downstreamFromStart.node() == m_downstreamEnd.node() &&
-        m_downstreamEnd.node()->enclosingBlockFlowElement() != upstreamFromBR.node()->enclosingBlockFlowElement();
-    if (startIsBRAfterBlock) {
-        removeNode(m_downstreamEnd.node());
-        m_endingPosition = upstreamFromBR;
-        m_mergeBlocksAfterDelete = false;
-        return true;
-    }
-
     // Not a special-case delete per se, but we can detect that the merging of content between blocks
     // should not be done.
     if (upstreamStartIsBR && downstreamStartIsBR)
@@ -2545,7 +2532,7 @@ void DeleteSelectionCommand::handleGeneralDelete()
         setStartNode(m_startNode->traverseNextNode());
     }
     else if (m_startBlock != m_endBlock && isStartOfBlock(VisiblePosition(m_upstreamStart, m_selectionToDelete.startAffinity()))) {
-        if (!isStartOfBlock(visibleEnd) && endAtEndOfBlock) {
+        if (!m_startBlock->isAncestor(m_endBlock) && !isStartOfBlock(visibleEnd) && endAtEndOfBlock) {
             // Delete all the children of the block, but not the block itself.
             setStartNode(m_startBlock->firstChild());
             startOffset = 0;