Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Mar 2005 18:43:36 +0000 (18:43 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Mar 2005 18:43:36 +0000 (18:43 +0000)
        Fix for this bug:

        <rdar://problem/4045521> Hitting return key with full line selected does not add blank line as it should

        * khtml/editing/htmlediting.cpp:
        (khtml::InsertParagraphSeparatorCommand::doApply): Removed some "special-case" code from this
        function that would look for a selection that started and ended in a different block, and would
        then bail right after the deletion of the selection without inserting a paragraph separator.
        This was just wrong. So, the code change is removal only. When the general-case code runs instead
        of the erroneous special-case code, the bug goes away.

        New tests:

        * layout-tests/editing/inserting/return-key-with-selection-001-expected.txt: Added.
        * layout-tests/editing/inserting/return-key-with-selection-001.html: Added.
        * layout-tests/editing/inserting/return-key-with-selection-002-expected.txt: Added.
        * layout-tests/editing/inserting/return-key-with-selection-002.html: Added.
        * layout-tests/editing/inserting/return-key-with-selection-003-expected.txt: Added.
        * layout-tests/editing/inserting/return-key-with-selection-003.html: Added.

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

LayoutTests/editing/inserting/return-key-with-selection-001-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/return-key-with-selection-001.html [new file with mode: 0644]
LayoutTests/editing/inserting/return-key-with-selection-002-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/return-key-with-selection-002.html [new file with mode: 0644]
LayoutTests/editing/inserting/return-key-with-selection-003-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/return-key-with-selection-003.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp

diff --git a/LayoutTests/editing/inserting/return-key-with-selection-001-expected.txt b/LayoutTests/editing/inserting/return-key-with-selection-001-expected.txt
new file mode 100644 (file)
index 0000000..603d0bc
--- /dev/null
@@ -0,0 +1,48 @@
+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 {DIV} at (0,0) size 784x296 [border: (2px solid #0000FF)]
+        RenderBlock {DIV} at (14,14) size 756x84
+          RenderText {TEXT} at (0,0) size 67x28
+            text run at (0,0) width 67: "Tests: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 162x28
+            text run at (0,28) width 162: "Fix for this bug: "
+          RenderInline {A} at (0,0) size 260x28 [color=#0000EE]
+            RenderText {TEXT} at (162,28) size 260x28
+              text run at (162,28) width 260: "<rdar://problem/4045521>"
+          RenderText {TEXT} at (422,28) size 734x56
+            text run at (422,28) width 312: " Hitting return key with full line"
+            text run at (0,56) width 426: "selected does not add blank line as it should"
+        RenderBlock {DIV} at (14,114) size 756x168
+          RenderBlock (anonymous) at (0,0) size 756x84
+            RenderText {TEXT} at (0,0) size 189x28
+              text run at (0,0) width 189: "Expected Results: "
+            RenderBR {BR} at (0,0) size 0x0
+            RenderText {TEXT} at (0,28) size 711x56
+              text run at (0,28) width 711: "Should see this content in the red box below (note that the insertion point"
+              text run at (0,56) width 670: "should be at the start of the third line, immediately preceding \"baz\"):"
+          RenderBlock {DIV} at (0,84) size 756x28
+            RenderText {TEXT} at (0,0) size 32x28
+              text run at (0,0) width 32: "foo"
+          RenderBlock {DIV} at (0,112) size 756x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (0,140) size 756x28
+            RenderText {TEXT} at (0,0) size 34x28
+              text run at (0,0) width 34: "baz"
+      RenderBlock {DIV} at (0,320) size 784x88
+        RenderBlock {DIV} at (0,0) size 784x88 [border: (2px solid #FF0000)]
+          RenderBlock {DIV} at (2,2) size 780x28
+            RenderText {TEXT} at (0,0) size 32x28
+              text run at (0,0) width 32: "foo"
+          RenderBlock {DIV} at (2,30) size 780x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (2,58) size 780x28
+            RenderText {TEXT} at (0,0) size 34x28
+              text run at (0,0) width 34: "baz"
+selection is CARET:
+start:      position 0 of child 1 {TEXT} of child 4 {DIV} of child 1 {DIV} of root {DIV}
+upstream:   position 0 of child 4 {DIV} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 1 {TEXT} of child 4 {DIV} of child 1 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/inserting/return-key-with-selection-001.html b/LayoutTests/editing/inserting/return-key-with-selection-001.html
new file mode 100644 (file)
index 0000000..7baa308
--- /dev/null
@@ -0,0 +1,61 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+.scenario { margin-bottom: 16px;}
+.scenario:first-line { font-weight: bold; margin-bottom: 16px;}
+.expected-results:first-line { font-weight: bold }
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    moveSelectionForwardByLineCommand();
+    extendSelectionForwardByLineCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+
+<div class="explanation">
+<div class="scenario">
+Tests: 
+<br>
+Fix for this bug: 
+<a href="rdar://problem/4045521">&lt;rdar://problem/4045521&gt;</a> Hitting return key with full line selected does not add blank line as it should
+</div>
+<div class="expected-results">
+Expected Results:
+<br>
+Should see this content in the red box below (note that the insertion point should be at the start of the third line, immediately preceding "baz"):
+<div>foo</div><div><br></div><div>baz</div>
+</div>
+</div>
+
+<div contenteditable id="root" style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+<div id="test" class="editing">
+<div>foo</div><div>bar</div><div>baz</div>
+</div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/return-key-with-selection-002-expected.txt b/LayoutTests/editing/inserting/return-key-with-selection-002-expected.txt
new file mode 100644 (file)
index 0000000..93b1fc1
--- /dev/null
@@ -0,0 +1,48 @@
+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 {DIV} at (0,0) size 784x324 [border: (2px solid #0000FF)]
+        RenderBlock {DIV} at (14,14) size 756x112
+          RenderText {TEXT} at (0,0) size 67x28
+            text run at (0,0) width 67: "Tests: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 612x28
+            text run at (0,28) width 612: "A scenario I thought of based on my experiences with this bug:"
+          RenderInline {A} at (0,0) size 260x28 [color=#0000EE]
+            RenderText {TEXT} at (0,56) size 260x28
+              text run at (0,56) width 260: "<rdar://problem/4045521>"
+          RenderText {TEXT} at (260,56) size 744x56
+            text run at (260,56) width 484: " Hitting return key with full line selected does not"
+            text run at (0,84) width 254: "add blank line as it should"
+        RenderBlock {DIV} at (14,142) size 756x168
+          RenderBlock (anonymous) at (0,0) size 756x84
+            RenderText {TEXT} at (0,0) size 189x28
+              text run at (0,0) width 189: "Expected Results: "
+            RenderBR {BR} at (0,0) size 0x0
+            RenderText {TEXT} at (0,28) size 711x56
+              text run at (0,28) width 711: "Should see this content in the red box below (note that the insertion point"
+              text run at (0,56) width 670: "should be at the start of the third line, immediately preceding \"baz\"):"
+          RenderBlock {DIV} at (0,84) size 756x28
+            RenderText {TEXT} at (0,0) size 32x28
+              text run at (0,0) width 32: "foo"
+          RenderBlock {DIV} at (0,112) size 756x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (0,140) size 756x28
+            RenderText {TEXT} at (0,0) size 34x28
+              text run at (0,0) width 34: "baz"
+      RenderBlock {DIV} at (0,348) size 784x88
+        RenderBlock {DIV} at (0,0) size 784x88 [border: (2px solid #FF0000)]
+          RenderBlock {DIV} at (2,2) size 780x28
+            RenderText {TEXT} at (0,0) size 32x28
+              text run at (0,0) width 32: "foo"
+          RenderBlock {DIV} at (2,30) size 780x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (2,58) size 780x28
+            RenderText {TEXT} at (0,0) size 34x28
+              text run at (0,0) width 34: "baz"
+selection is CARET:
+start:      position 0 of child 1 {TEXT} of child 4 {DIV} of child 1 {DIV} of root {DIV}
+upstream:   position 0 of child 4 {DIV} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 1 {TEXT} of child 4 {DIV} of child 1 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/inserting/return-key-with-selection-002.html b/LayoutTests/editing/inserting/return-key-with-selection-002.html
new file mode 100644 (file)
index 0000000..5fa9871
--- /dev/null
@@ -0,0 +1,62 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+.scenario { margin-bottom: 16px;}
+.scenario:first-line { font-weight: bold; margin-bottom: 16px;}
+.expected-results:first-line { font-weight: bold }
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    moveSelectionForwardByLineCommand();
+    extendSelectionForwardByLineCommand();
+    extendSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+
+<div class="explanation">
+<div class="scenario">
+Tests: 
+<br>
+A scenario I thought of based on my experiences with this bug: 
+<a href="rdar://problem/4045521">&lt;rdar://problem/4045521&gt;</a> Hitting return key with full line selected does not add blank line as it should
+</div>
+<div class="expected-results">
+Expected Results:
+<br>
+Should see this content in the red box below (note that the insertion point should be at the start of the third line, immediately preceding "baz"):
+<div>foo</div><div><br></div><div>baz</div>
+</div>
+</div>
+
+<div contenteditable id="root" style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+<div id="test" class="editing">
+<div>foo</div><div>bar</div><div>bbaz</div>
+</div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/return-key-with-selection-003-expected.txt b/LayoutTests/editing/inserting/return-key-with-selection-003-expected.txt
new file mode 100644 (file)
index 0000000..36a0643
--- /dev/null
@@ -0,0 +1,53 @@
+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 {DIV} at (0,0) size 784x380 [border: (2px solid #0000FF)]
+        RenderBlock {DIV} at (14,14) size 756x112
+          RenderText {TEXT} at (0,0) size 67x28
+            text run at (0,0) width 67: "Tests: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 612x28
+            text run at (0,28) width 612: "A scenario I thought of based on my experiences with this bug:"
+          RenderInline {A} at (0,0) size 260x28 [color=#0000EE]
+            RenderText {TEXT} at (0,56) size 260x28
+              text run at (0,56) width 260: "<rdar://problem/4045521>"
+          RenderText {TEXT} at (260,56) size 744x56
+            text run at (260,56) width 484: " Hitting return key with full line selected does not"
+            text run at (0,84) width 254: "add blank line as it should"
+        RenderBlock {DIV} at (14,142) size 756x224
+          RenderBlock (anonymous) at (0,0) size 756x112
+            RenderText {TEXT} at (0,0) size 189x28
+              text run at (0,0) width 189: "Expected Results: "
+            RenderBR {BR} at (0,0) size 0x0
+            RenderText {TEXT} at (0,28) size 748x84
+              text run at (0,28) width 724: "Should see this content in the red box below (note that there should be two"
+              text run at (0,56) width 748: "blank lines between \"foo\" and \"baz\"; also note that the insertion point should"
+              text run at (0,84) width 429: "be at the start of the third line, a blank line):"
+          RenderBlock {DIV} at (0,112) size 756x28
+            RenderText {TEXT} at (0,0) size 32x28
+              text run at (0,0) width 32: "foo"
+          RenderBlock {DIV} at (0,140) size 756x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (0,168) size 756x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (0,196) size 756x28
+            RenderText {TEXT} at (0,0) size 34x28
+              text run at (0,0) width 34: "baz"
+      RenderBlock {DIV} at (0,404) size 784x116
+        RenderBlock {DIV} at (0,0) size 784x116 [border: (2px solid #FF0000)]
+          RenderBlock {DIV} at (2,2) size 780x28
+            RenderText {TEXT} at (0,0) size 32x28
+              text run at (0,0) width 32: "foo"
+          RenderBlock {DIV} at (2,30) size 780x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (2,58) size 780x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (2,86) size 780x28
+            RenderText {TEXT} at (0,0) size 34x28
+              text run at (0,0) width 34: "baz"
+selection is CARET:
+start:      position 0 of child 1 {BR} of child 4 {DIV} of child 1 {DIV} of root {DIV}
+upstream:   position 0 of child 4 {DIV} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 1 {BR} of child 4 {DIV} of child 1 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/inserting/return-key-with-selection-003.html b/LayoutTests/editing/inserting/return-key-with-selection-003.html
new file mode 100644 (file)
index 0000000..be69d77
--- /dev/null
@@ -0,0 +1,62 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+.scenario { margin-bottom: 16px;}
+.scenario:first-line { font-weight: bold; margin-bottom: 16px;}
+.expected-results:first-line { font-weight: bold }
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    moveSelectionForwardByLineCommand();
+    for (i = 0; i < 3; i++)
+        extendSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+
+<div class="explanation">
+<div class="scenario">
+Tests: 
+<br>
+A scenario I thought of based on my experiences with this bug: 
+<a href="rdar://problem/4045521">&lt;rdar://problem/4045521&gt;</a> Hitting return key with full line selected does not add blank line as it should
+</div>
+<div class="expected-results">
+Expected Results:
+<br>
+Should see this content in the red box below (note that there should be two blank lines between "foo" and "baz"; also note that the insertion point should be at the start of the third line, a blank line):
+<div>foo</div><div><br></div><div><br></div><div>baz</div>
+</div>
+</div>
+
+<div contenteditable id="root" style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+<div id="test" class="editing">
+<div>foo</div><div>bar</div><div>baz</div>
+</div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 49ebb4d041389f31775b4be1268b4e988a91ad4d..979dfd1461ea2b756ba87fa076cd7b35a54e9644 100644 (file)
@@ -1,3 +1,27 @@
+2005-03-11  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        Fix for this bug:
+        
+        <rdar://problem/4045521> Hitting return key with full line selected does not add blank line as it should
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::InsertParagraphSeparatorCommand::doApply): Removed some "special-case" code from this 
+        function that would look for a selection that started and ended in a different block, and would
+        then bail right after the deletion of the selection without inserting a paragraph separator.
+        This was just wrong. So, the code change is removal only. When the general-case code runs instead
+        of the erroneous special-case code, the bug goes away.
+        
+        New tests:
+        
+        * layout-tests/editing/inserting/return-key-with-selection-001-expected.txt: Added.
+        * layout-tests/editing/inserting/return-key-with-selection-001.html: Added.
+        * layout-tests/editing/inserting/return-key-with-selection-002-expected.txt: Added.
+        * layout-tests/editing/inserting/return-key-with-selection-002.html: Added.
+        * layout-tests/editing/inserting/return-key-with-selection-003-expected.txt: Added.
+        * layout-tests/editing/inserting/return-key-with-selection-003.html: Added.
+
 2005-03-11  David Harrison  <harrison@apple.com>
 
         Reviewed by Darin.
index e599c00a7375c94497f21771be559bda4a5372f7..c184124044f4d8b7c981a761b0f2b5fa859b576d 100644 (file)
@@ -3198,27 +3198,13 @@ void InsertParagraphSeparatorCommand::doApply()
     if (selection.isNone())
         return;
     
-    // Delete the current selection.
-    // If the selection is a range and the start and end nodes are in different blocks, 
-    // then this command bails after the delete, but takes the one additional step of
-    // moving the selection downstream so it is in the ending block (if that block is
-    // still around, that is).
     Position pos = selection.start();
     EAffinity affinity = selection.startAffinity();
         
+    // Delete the current selection.
     if (selection.isRange()) {
-        NodeImpl *startBlockBeforeDelete = selection.start().node()->enclosingBlockFlowElement();
-        NodeImpl *endBlockBeforeDelete = selection.end().node()->enclosingBlockFlowElement();
-        bool doneAfterDelete = startBlockBeforeDelete != endBlockBeforeDelete;
         calculateStyleBeforeInsertion(pos);
         deleteSelection(false, false);
-        if (doneAfterDelete) {
-            document()->updateLayout();
-            setEndingSelection(endingSelection().start().downstream(), DOWNSTREAM);
-            rebalanceWhitespace();
-            applyStyleAfterInsertion();
-            return;
-        }
         pos = endingSelection().start();
         affinity = endingSelection().startAffinity();
     }