Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Dec 2004 18:46:13 +0000 (18:46 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Dec 2004 18:46:13 +0000 (18:46 +0000)
        Fix for this bug:

        <rdar://problem/3918351> REGRESSION (Mail, 173-175+): Return before first char of line leaves insertion point in wrong place

        * khtml/editing/htmlediting.cpp:
        (khtml::InsertParagraphSeparatorCommand::doApply): Basically, did a rewrite of this function
        to do a better job than it was doing before. Added several test cases to prove I am on a
        better track.
        * khtml/editing/visible_position.cpp:
        (khtml::isFirstVisiblePositionInBlock): Tweaked the rules a bit to fix an issue very similar to the
        leaving-the-bar-node case problem I just fixed in a recent checkin (relevant markup: <p>foo</p>bar).
        This function was returning true for the first position in "bar". Wrong. Also tightened up other
        rule: Should not report true when relationship between blocks cannot be determined.
        (khtml::isLastVisiblePositionInBlock): Tightened up rule as above: Should not report true
        when relationship between blocks cannot be determined.
        * layout-tests/editing/inserting/insert-div-010-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-010.html: Added.
        * layout-tests/editing/inserting/insert-div-011-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-011.html: Added.
        * layout-tests/editing/inserting/insert-div-012-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-012.html: Added.
        * layout-tests/editing/inserting/insert-div-013-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-013.html: Added.
        * layout-tests/editing/inserting/insert-div-014-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-014.html: Added.
        * layout-tests/editing/inserting/insert-div-015-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-015.html: Added.
        * layout-tests/editing/inserting/insert-div-016-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-016.html: Added.
        * layout-tests/editing/inserting/insert-div-017-expected.txt: Added.
        * layout-tests/editing/inserting/insert-div-017.html: Added.

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

19 files changed:
LayoutTests/editing/inserting/insert-div-010-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-010.html [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-011-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-011.html [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-012-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-012.html [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-013-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-013.html [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-014-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-014.html [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-015-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-015.html [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-016-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-016.html [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-017-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-017.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/visible_position.cpp

diff --git a/LayoutTests/editing/inserting/insert-div-010-expected.txt b/LayoutTests/editing/inserting/insert-div-010-expected.txt
new file mode 100644 (file)
index 0000000..2686270
--- /dev/null
@@ -0,0 +1,14 @@
+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 [border: (2px solid #FF0000)]
+      RenderBlock {DIV} at (14,14) size 756x28
+        RenderBR {BR} at (0,0) size 0x28
+      RenderBlock {DIV} at (14,42) size 756x28
+        RenderText {TEXT} at (0,0) size 12x28
+          text run at (0,0) width 12: "x"
+selection is CARET:
+start:      position 0 of child 1 {TEXT} of child 2 {DIV} of root {BODY}
+upstream:   position 0 of child 2 {DIV} of root {BODY}
+downstream: position 0 of child 1 {TEXT} of child 2 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-010.html b/LayoutTests/editing/inserting/insert-div-010.html
new file mode 100644 (file)
index 0000000..de2df62
--- /dev/null
@@ -0,0 +1,36 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    typeCharacterCommand();
+    moveSelectionBackwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root" class="editing">
+<div id="test"><br class="khtml-block-placeholder"></div>
+
+<!-- Test fix for <rdar://problem/3918351> REGRESSION (Mail, 173-175+): Return 
+     before first char of line leaves insertion point in wrong place -->
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/insert-div-011-expected.txt b/LayoutTests/editing/inserting/insert-div-011-expected.txt
new file mode 100644 (file)
index 0000000..60e75f0
--- /dev/null
@@ -0,0 +1,25 @@
+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 (anonymous) at (0,0) size 784x28
+        RenderText {TEXT} at (0,0) size 629x28
+          text run at (0,0) width 629: "Test inserting paragraphs: should see empty red box above \"baz\""
+      RenderBlock {DIV} at (0,28) size 784x12
+      RenderBlock {DIV} at (0,40) size 784x28 [border: (2px solid #FF0000)]
+      RenderBlock {DIV} at (0,68) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 34x28
+          text run at (14,14) width 34: "baz"
+      RenderBlock (anonymous) at (0,124) size 784x28
+        RenderText {TEXT} at (0,0) size 31x28
+          text run at (0,0) width 31: "bar"
+      RenderBlock {DIV} at (0,152) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 32x28
+          text run at (14,14) width 32: "foo"
+      RenderBlock {DIV} at (0,208) size 784x56 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (14,14) size 0x28
+selection is CARET:
+start:      position 0 of child 1 {TEXT} of child 5 {DIV} of root {BODY}
+upstream:   position 0 of child 5 {DIV} of root {BODY}
+downstream: position 0 of child 1 {TEXT} of child 5 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-011.html b/LayoutTests/editing/inserting/insert-div-011.html
new file mode 100644 (file)
index 0000000..1886d73
--- /dev/null
@@ -0,0 +1,40 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see empty red box above "baz"
+
+<div style="height: 12px"></div>
+
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/insert-div-012-expected.txt b/LayoutTests/editing/inserting/insert-div-012-expected.txt
new file mode 100644 (file)
index 0000000..e85a149
--- /dev/null
@@ -0,0 +1,25 @@
+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 (anonymous) at (0,0) size 784x28
+        RenderText {TEXT} at (0,0) size 630x28
+          text run at (0,0) width 630: "Test inserting paragraphs: should see empty red box below \"baz\""
+      RenderBlock {DIV} at (0,28) size 784x12
+      RenderBlock {DIV} at (0,40) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 34x28
+          text run at (14,14) width 34: "baz"
+      RenderBlock {DIV} at (0,96) size 784x28 [border: (2px solid #FF0000)]
+      RenderBlock (anonymous) at (0,124) size 784x28
+        RenderText {TEXT} at (0,0) size 31x28
+          text run at (0,0) width 31: "bar"
+      RenderBlock {DIV} at (0,152) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 32x28
+          text run at (14,14) width 32: "foo"
+      RenderBlock {DIV} at (0,208) size 784x56 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (14,14) size 0x28
+selection is CARET:
+start:      position 0 of child 5 {DIV} of root {BODY}
+upstream:   position 0 of child 5 {DIV} of root {BODY}
+downstream: position 1 of child 5 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-012.html b/LayoutTests/editing/inserting/insert-div-012.html
new file mode 100644 (file)
index 0000000..43d01e4
--- /dev/null
@@ -0,0 +1,46 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+p {
+    border: 2px solid blue; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 3; i++)
+        moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see empty red box below "baz"
+
+<div style="height: 12px"></div>
+
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/insert-div-013-expected.txt b/LayoutTests/editing/inserting/insert-div-013-expected.txt
new file mode 100644 (file)
index 0000000..3d807f1
--- /dev/null
@@ -0,0 +1,25 @@
+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 (anonymous) at (0,0) size 784x28
+        RenderText {TEXT} at (0,0) size 637x28
+          text run at (0,0) width 637: "Test inserting paragraphs: should see empty blue box above \"bar\""
+      RenderBlock {DIV} at (0,28) size 784x12
+      RenderBlock {DIV} at (0,40) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 34x28
+          text run at (14,14) width 34: "baz"
+      RenderBlock {P} at (0,98) size 784x28 [border: (2px solid #0000FF)]
+      RenderBlock (anonymous) at (0,128) size 784x28
+        RenderText {TEXT} at (0,0) size 31x28
+          text run at (0,0) width 31: "bar"
+      RenderBlock {DIV} at (0,156) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 32x28
+          text run at (14,14) width 32: "foo"
+      RenderBlock {DIV} at (0,212) size 784x56 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (14,14) size 0x28
+selection is CARET:
+start:      position 0 of child 6 {TEXT} of root {BODY}
+upstream:   position 0 of child 6 {TEXT} of root {BODY}
+downstream: position 0 of child 6 {TEXT} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-013.html b/LayoutTests/editing/inserting/insert-div-013.html
new file mode 100644 (file)
index 0000000..3cb2bc9
--- /dev/null
@@ -0,0 +1,46 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+p {
+    border: 2px solid blue; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 4; i++)
+        moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see empty blue box above "bar"
+
+<div style="height: 12px"></div>
+
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/insert-div-014-expected.txt b/LayoutTests/editing/inserting/insert-div-014-expected.txt
new file mode 100644 (file)
index 0000000..c210bf9
--- /dev/null
@@ -0,0 +1,25 @@
+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 (anonymous) at (0,0) size 784x28
+        RenderText {TEXT} at (0,0) size 638x28
+          text run at (0,0) width 638: "Test inserting paragraphs: should see empty blue box below \"bar\""
+      RenderBlock {DIV} at (0,28) size 784x12
+      RenderBlock {DIV} at (0,40) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 34x28
+          text run at (14,14) width 34: "baz"
+      RenderBlock (anonymous) at (0,96) size 784x28
+        RenderText {TEXT} at (0,0) size 31x28
+          text run at (0,0) width 31: "bar"
+      RenderBlock {P} at (0,126) size 784x28 [border: (2px solid #0000FF)]
+      RenderBlock {DIV} at (0,156) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 32x28
+          text run at (14,14) width 32: "foo"
+      RenderBlock {DIV} at (0,212) size 784x56 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (14,14) size 0x28
+selection is CARET:
+start:      position 0 of child 6 {P} of root {BODY}
+upstream:   position 0 of child 6 {P} of root {BODY}
+downstream: position 1 of child 6 {P} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-014.html b/LayoutTests/editing/inserting/insert-div-014.html
new file mode 100644 (file)
index 0000000..ae0e0ff
--- /dev/null
@@ -0,0 +1,46 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+p {
+    border: 2px solid blue; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 7; i++)
+        moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see empty blue box below "bar"
+
+<div style="height: 12px"></div>
+
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/insert-div-015-expected.txt b/LayoutTests/editing/inserting/insert-div-015-expected.txt
new file mode 100644 (file)
index 0000000..8864017
--- /dev/null
@@ -0,0 +1,25 @@
+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 (anonymous) at (0,0) size 784x28
+        RenderText {TEXT} at (0,0) size 627x28
+          text run at (0,0) width 627: "Test inserting paragraphs: should see empty red box above \"foo\""
+      RenderBlock {DIV} at (0,28) size 784x12
+      RenderBlock {DIV} at (0,40) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 34x28
+          text run at (14,14) width 34: "baz"
+      RenderBlock (anonymous) at (0,96) size 784x28
+        RenderText {TEXT} at (0,0) size 31x28
+          text run at (0,0) width 31: "bar"
+      RenderBlock {DIV} at (0,124) size 784x28 [border: (2px solid #FF0000)]
+      RenderBlock {DIV} at (0,152) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 32x28
+          text run at (14,14) width 32: "foo"
+      RenderBlock {DIV} at (0,208) size 784x56 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (14,14) size 0x28
+selection is CARET:
+start:      position 0 of child 1 {TEXT} of child 7 {DIV} of root {BODY}
+upstream:   position 0 of child 7 {DIV} of root {BODY}
+downstream: position 0 of child 1 {TEXT} of child 7 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-015.html b/LayoutTests/editing/inserting/insert-div-015.html
new file mode 100644 (file)
index 0000000..76e56ef
--- /dev/null
@@ -0,0 +1,46 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+p {
+    border: 2px solid blue; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 8; i++)
+        moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see empty red box above "foo"
+
+<div style="height: 12px"></div>
+
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/insert-div-016-expected.txt b/LayoutTests/editing/inserting/insert-div-016-expected.txt
new file mode 100644 (file)
index 0000000..dc9d8dc
--- /dev/null
@@ -0,0 +1,25 @@
+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 (anonymous) at (0,0) size 784x28
+        RenderText {TEXT} at (0,0) size 690x28
+          text run at (0,0) width 690: "Test inserting paragraphs: should see two empty red boxes below \"foo\""
+      RenderBlock {DIV} at (0,28) size 784x12
+      RenderBlock {DIV} at (0,40) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 34x28
+          text run at (14,14) width 34: "baz"
+      RenderBlock (anonymous) at (0,96) size 784x28
+        RenderText {TEXT} at (0,0) size 31x28
+          text run at (0,0) width 31: "bar"
+      RenderBlock {DIV} at (0,124) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 32x28
+          text run at (14,14) width 32: "foo"
+      RenderBlock {DIV} at (0,180) size 784x28 [border: (2px solid #FF0000)]
+      RenderBlock {DIV} at (0,208) size 784x56 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (14,14) size 0x28
+selection is CARET:
+start:      position 0 of child 7 {DIV} of root {BODY}
+upstream:   position 0 of child 7 {DIV} of root {BODY}
+downstream: position 1 of child 7 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-016.html b/LayoutTests/editing/inserting/insert-div-016.html
new file mode 100644 (file)
index 0000000..0f23008
--- /dev/null
@@ -0,0 +1,46 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+p {
+    border: 2px solid blue; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 11; i++)
+        moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see two empty red boxes below "foo"
+
+<div style="height: 12px"></div>
+
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/insert-div-017-expected.txt b/LayoutTests/editing/inserting/insert-div-017-expected.txt
new file mode 100644 (file)
index 0000000..df63bee
--- /dev/null
@@ -0,0 +1,25 @@
+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 (anonymous) at (0,0) size 784x28
+        RenderText {TEXT} at (0,0) size 690x28
+          text run at (0,0) width 690: "Test inserting paragraphs: should see two empty red boxes below \"foo\""
+      RenderBlock {DIV} at (0,28) size 784x12
+      RenderBlock {DIV} at (0,40) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 34x28
+          text run at (14,14) width 34: "baz"
+      RenderBlock (anonymous) at (0,96) size 784x28
+        RenderText {TEXT} at (0,0) size 31x28
+          text run at (0,0) width 31: "bar"
+      RenderBlock {DIV} at (0,124) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 32x28
+          text run at (14,14) width 32: "foo"
+      RenderBlock {DIV} at (0,180) size 784x56 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (14,14) size 0x28
+      RenderBlock {DIV} at (0,236) size 784x28 [border: (2px solid #FF0000)]
+selection is CARET:
+start:      position 0 of child 9 {DIV} of root {BODY}
+upstream:   position 0 of child 9 {DIV} of root {BODY}
+downstream: position 1 of child 9 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-017.html b/LayoutTests/editing/inserting/insert-div-017.html
new file mode 100644 (file)
index 0000000..634e32d
--- /dev/null
@@ -0,0 +1,46 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+p {
+    border: 2px solid blue; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 12; i++)
+        moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see two empty red boxes below "foo"
+
+<div style="height: 12px"></div>
+
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index c9b259c..0fcca79 100644 (file)
@@ -1,5 +1,41 @@
 2004-12-16  Ken Kocienda  <kocienda@apple.com>
 
+        Reviewed by John
+        
+        Fix for this bug:
+        
+        <rdar://problem/3918351> REGRESSION (Mail, 173-175+): Return before first char of line leaves insertion point in wrong place
+        
+        * khtml/editing/htmlediting.cpp:
+        (khtml::InsertParagraphSeparatorCommand::doApply): Basically, did a rewrite of this function
+        to do a better job than it was doing before. Added several test cases to prove I am on a 
+        better track.
+        * khtml/editing/visible_position.cpp:
+        (khtml::isFirstVisiblePositionInBlock): Tweaked the rules a bit to fix an issue very similar to the
+        leaving-the-bar-node case problem I just fixed in a recent checkin (relevant markup: <p>foo</p>bar).
+        This function was returning true for the first position in "bar". Wrong. Also tightened up other 
+        rule: Should not report true when relationship between blocks cannot be determined.
+        (khtml::isLastVisiblePositionInBlock): Tightened up rule as above: Should not report true 
+        when relationship between blocks cannot be determined.
+        * layout-tests/editing/inserting/insert-div-010-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-010.html: Added.
+        * layout-tests/editing/inserting/insert-div-011-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-011.html: Added.
+        * layout-tests/editing/inserting/insert-div-012-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-012.html: Added.
+        * layout-tests/editing/inserting/insert-div-013-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-013.html: Added.
+        * layout-tests/editing/inserting/insert-div-014-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-014.html: Added.
+        * layout-tests/editing/inserting/insert-div-015-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-015.html: Added.
+        * layout-tests/editing/inserting/insert-div-016-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-016.html: Added.
+        * layout-tests/editing/inserting/insert-div-017-expected.txt: Added.
+        * layout-tests/editing/inserting/insert-div-017.html: Added.
+            
+2004-12-16  Ken Kocienda  <kocienda@apple.com>
+
         Reviewed by me
         
         Added a layout test based on my last checkin.
index 10a0cf7..1ee13ea 100644 (file)
@@ -2217,109 +2217,139 @@ void InsertParagraphSeparatorCommand::doApply()
             rebalanceWhitespace();
             return;
         }
-        pos = endingSelection().start().upstream();
+        pos = endingSelection().start();
     }
-    
+
     // Find the start block.
     NodeImpl *startNode = pos.node();
     NodeImpl *startBlock = startNode->enclosingBlockFlowElement();
     if (!startBlock || !startBlock->parentNode())
         return;
 
-    // Build up list of ancestors in between the start node and the start block.
-    if (startNode != startBlock) {
-        for (NodeImpl *n = startNode->parentNode(); n && n != startBlock; n = n->parentNode())
-            ancestors.prepend(n);
-    }
+    VisiblePosition visiblePos(pos);
+    bool isFirstInBlock = isFirstVisiblePositionInBlock(visiblePos);
+    bool isLastInBlock = isLastVisiblePositionInBlock(visiblePos);
+    bool startBlockIsRoot = startBlock == startBlock->rootEditableElement();
 
-    // Make new block to represent the newline.
-    // If the start block is the body, just make a P tag, otherwise, make a shallow clone
-    // of the the start block.
-    NodeImpl *addedBlock = 0;
-    if (startBlock == startBlock->rootEditableElement()) {
-        if (startBlock->renderer() && !startBlock->renderer()->firstChild()) {
-            // No rendered kids in the root editable element.
-            // Just inserting a <p> in this situation is not enough, since this operation
-            // is supposed to add an additional user-visible line to the content.
-            // So, insert an extra <p> to make the one we insert right appear as the second
-            // line in the root editable element.
+    // This is the block that is going to be inserted.
+    NodeImpl *blockToInsert = startBlockIsRoot ? createParagraphElement() : startBlock->cloneNode(false);
+    
+    //---------------------------------------------------------------------
+    // Handle empty block case.
+    if (isFirstInBlock && isLastInBlock) {
+        LOG(Editing, "insert paragraph separator: empty block case");
+        if (startBlockIsRoot) {
             NodeImpl *extraBlock = createParagraphElement();
             appendNode(extraBlock, startBlock);
             insertBlockPlaceholderIfNeeded(extraBlock);
+            appendNode(blockToInsert, startBlock);
         }
-        addedBlock = createParagraphElement();
-        appendNode(addedBlock, startBlock);
+        else {
+            insertNodeAfter(blockToInsert, startBlock);
+        }
+        insertBlockPlaceholderIfNeeded(blockToInsert);
+        setEndingSelection(Position(blockToInsert, 0));
+        return;
     }
-    else {
-        addedBlock = startBlock->cloneNode(false);
-        insertNodeAfter(addedBlock, startBlock);
+
+    //---------------------------------------------------------------------
+    // Handle case when position is in the first visible position in its block.
+    // and similar case where upstream position is in another block.
+    bool upstreamInDifferentBlock = startBlock != pos.upstream(DoNotStayInBlock).node()->enclosingBlockFlowElement();
+    if (upstreamInDifferentBlock || isFirstInBlock) {
+        LOG(Editing, "insert paragraph separator: first in block case");
+        pos = pos.downstream(StayInBlock);
+        insertNodeBefore(blockToInsert, startBlockIsRoot ? pos.node() : startBlock);
+        insertBlockPlaceholderIfNeeded(blockToInsert);
+        setEndingSelection(pos);
+        return;
     }
-    addedBlock->ref();
-    insertBlockPlaceholderIfNeeded(addedBlock);
-    clonedNodes.append(addedBlock);
 
-    if (!isLastVisiblePositionInNode(VisiblePosition(pos), startBlock)) {
-        // Split at pos if in the middle of a text node.
-        if (startNode->isTextNode()) {
-            TextImpl *textNode = static_cast<TextImpl *>(startNode);
-            bool atEnd = (unsigned long)pos.offset() >= textNode->length();
-            if (pos.offset() > 0 && !atEnd) {
-                SplitTextNodeCommand *splitCommand = new SplitTextNodeCommand(document(), textNode, pos.offset());
-                EditCommandPtr cmd(splitCommand);
-                applyCommandToComposite(cmd);
-                startNode = splitCommand->node();
-                pos = Position(startNode, 0);
-            }
-            else if (atEnd) {
-                startNode = startNode->traverseNextNode();
-                ASSERT(startNode);
-            }
+    //---------------------------------------------------------------------
+    // Handle case when position is in the last visible position in its block, 
+    // and similar case where downstream position is in another block.
+    bool downstreamInDifferentBlock = startBlock != pos.downstream(DoNotStayInBlock).node()->enclosingBlockFlowElement();
+    if (downstreamInDifferentBlock || isLastInBlock) {
+        LOG(Editing, "insert paragraph separator: last in block case");
+        insertNodeAfter(blockToInsert, startBlockIsRoot ? pos.node() : startBlock);
+        insertBlockPlaceholderIfNeeded(blockToInsert);
+        setEndingSelection(Position(blockToInsert, 0));
+        return;
+    }
+
+    //---------------------------------------------------------------------
+    // Handle the (more complicated) general case,
+
+    LOG(Editing, "insert paragraph separator: general case");
+
+    // Put the added block in the tree.
+    insertNodeAfter(blockToInsert, startBlockIsRoot ? pos.node() : startBlock);
+
+    // Build up list of ancestors in between the start node and the start block.
+    if (startNode != startBlock) {
+        for (NodeImpl *n = startNode->parentNode(); n && n != startBlock; n = n->parentNode())
+            ancestors.prepend(n);
+    }
+
+    // Split at pos if in the middle of a text node.
+    if (startNode->isTextNode()) {
+        TextImpl *textNode = static_cast<TextImpl *>(startNode);
+        bool atEnd = (unsigned long)pos.offset() >= textNode->length();
+        if (pos.offset() > 0 && !atEnd) {
+            SplitTextNodeCommand *splitCommand = new SplitTextNodeCommand(document(), textNode, pos.offset());
+            EditCommandPtr cmd(splitCommand);
+            applyCommandToComposite(cmd);
+            startNode = splitCommand->node();
+            pos = Position(startNode, 0);
         }
-        else if (pos.offset() > 0) {
+        else if (atEnd) {
             startNode = startNode->traverseNextNode();
             ASSERT(startNode);
         }
+    }
+    else if (pos.offset() > 0) {
+        startNode = startNode->traverseNextNode();
+        ASSERT(startNode);
+    }
 
-        // Make clones of ancestors in between the start node and the start block.
-        NodeImpl *parent = addedBlock;
-        for (QPtrListIterator<NodeImpl> it(ancestors); it.current(); ++it) {
-            NodeImpl *child = it.current()->cloneNode(false); // shallow clone
-            child->ref();
-            clonedNodes.append(child);
-            appendNode(child, parent);
-            parent = child;
-        }
-
-        // Move the start node and the siblings of the start node.
-        if (startNode != startBlock) {
-            NodeImpl *n = startNode;
-            if (n->id() == ID_BR)
-                n = n->nextSibling();
-            while (n && n != addedBlock) {
-                NodeImpl *next = n->nextSibling();
-                removeNode(n);
-                appendNode(n, parent);
-                n = next;
-            }
-        }            
-
-        // Move everything after the start node.
-        NodeImpl *leftParent = ancestors.last();
-        while (leftParent && leftParent != startBlock) {
-            parent = parent->parentNode();
-            NodeImpl *n = leftParent->nextSibling();
-            while (n) {
-                NodeImpl *next = n->nextSibling();
-                removeNode(n);
-                appendNode(n, parent);
-                n = next;
-            }
-            leftParent = leftParent->parentNode();
-        }
+    // Make clones of ancestors in between the start node and the start block.
+    NodeImpl *parent = blockToInsert;
+    for (QPtrListIterator<NodeImpl> it(ancestors); it.current(); ++it) {
+        NodeImpl *child = it.current()->cloneNode(false); // shallow clone
+        child->ref();
+        clonedNodes.append(child);
+        appendNode(child, parent);
+        parent = child;
     }
-    
-    // Put the selection right at the start of the added block.
-    setEndingSelection(Position(addedBlock, 0));
+
+    // Move the start node and the siblings of the start node.
+    if (startNode != startBlock) {
+        NodeImpl *n = startNode;
+        if (n->id() == ID_BR)
+            n = n->nextSibling();
+        while (n && n != blockToInsert) {
+            NodeImpl *next = n->nextSibling();
+            removeNode(n);
+            appendNode(n, parent);
+            n = next;
+        }
+    }            
+
+    // Move everything after the start node.
+    NodeImpl *leftParent = ancestors.last();
+    while (leftParent && leftParent != startBlock) {
+        parent = parent->parentNode();
+        NodeImpl *n = leftParent->nextSibling();
+        while (n) {
+            NodeImpl *next = n->nextSibling();
+            removeNode(n);
+            appendNode(n, parent);
+            n = next;
+        }
+        leftParent = leftParent->parentNode();
+    }
+
+    setEndingSelection(Position(blockToInsert, 0));
     rebalanceWhitespace();
 }
 
index 5460455..e47d6c6 100644 (file)
@@ -544,10 +544,10 @@ bool isFirstVisiblePositionInBlock(const VisiblePosition &pos)
         case NoBlockRelationship:
         case SameBlockRelationship:
         case AncestorBlockRelationship:
+        case OtherBlockRelationship:
             return false;
         case PeerBlockRelationship:
         case DescendantBlockRelationship:
-        case OtherBlockRelationship:
             return true;
     }
     ASSERT_NOT_REACHED();
@@ -584,11 +584,11 @@ bool isLastVisiblePositionInBlock(const VisiblePosition &pos)
     switch (blockRelationship(pos, next)) {
         case NoBlockRelationship:
         case SameBlockRelationship:
+        case AncestorBlockRelationship:
         case DescendantBlockRelationship:
+        case OtherBlockRelationship:
             return false;
         case PeerBlockRelationship:
-        case AncestorBlockRelationship:
-        case OtherBlockRelationship:
             return true;
     }
     ASSERT_NOT_REACHED();