2010-03-09 Tony Chang <tony@chromium.org>
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Mar 2010 05:32:21 +0000 (05:32 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Mar 2010 05:32:21 +0000 (05:32 +0000)
        Reviewed by Adam Barth.

        https://bugs.webkit.org/show_bug.cgi?id=21840
        https://bugs.webkit.org/show_bug.cgi?id=23993

        Fix an editing bug where replacing a selection would result in the
        new text ending up inside nodes that were not visibly included in the
        selection.  Instead, move our destination position out of nodes that
        were not visibly included.

        Two new tests to verify the new behavior.  Because we're now inserting
        outside of some formatting nodes, some span tags are no longer necessary
        for undoing formatting caused by these formatting nodes.

        * editing/deleting/backspace-avoid-preceding-style-expected.txt: Added.
        * editing/deleting/backspace-avoid-preceding-style.html: Added.
        * editing/inserting/replace-at-visible-boundary-expected.txt: Added.
        * editing/inserting/replace-at-visible-boundary.html: Added.
        * platform/mac/editing/deleting/delete-3857753-fix-expected.txt:
        * platform/mac/editing/inserting/insert-div-026-expected.txt:
        * platform/mac/editing/pasteboard/paste-text-at-tabspan-001-expected.txt:
        * platform/mac/editing/pasteboard/paste-text-at-tabspan-002-expected.txt:
        * platform/mac/editing/style/font-family-with-space-expected.txt:
        * platform/mac/editing/style/smoosh-styles-001-expected.txt:
        * platform/mac/editing/style/style-boundary-005-expected.txt:
2010-03-09  Tony Chang  <tony@chromium.org>

        Reviewed by Adam Barth.

        https://bugs.webkit.org/show_bug.cgi?id=21840
        https://bugs.webkit.org/show_bug.cgi?id=23993

        Fix an editing bug where replacing a selection would result in the
        new text ending up inside nodes that were not visibly included in the
        selection.  Instead, move our destination position out of nodes that
        were not visibly included.

        Tests: editing/deleting/backspace-avoid-preceding-style.html
               editing/inserting/replace-at-visible-boundary.html

        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::positionAvoidingPrecedingNodes):
        (WebCore::ReplaceSelectionCommand::doApply):

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/deleting/backspace-avoid-preceding-style-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/backspace-avoid-preceding-style.html [new file with mode: 0644]
LayoutTests/editing/inserting/replace-at-visible-boundary-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/replace-at-visible-boundary.html [new file with mode: 0644]
LayoutTests/platform/mac/editing/deleting/delete-3857753-fix-expected.txt
LayoutTests/platform/mac/editing/inserting/insert-div-026-expected.txt
LayoutTests/platform/mac/editing/pasteboard/paste-text-at-tabspan-001-expected.txt
LayoutTests/platform/mac/editing/pasteboard/paste-text-at-tabspan-002-expected.txt
LayoutTests/platform/mac/editing/style/font-family-with-space-expected.txt
LayoutTests/platform/mac/editing/style/smoosh-styles-001-expected.txt
LayoutTests/platform/mac/editing/style/style-boundary-005-expected.txt
WebCore/ChangeLog
WebCore/editing/ReplaceSelectionCommand.cpp

index ba07895..e4ef7d8 100644 (file)
@@ -1,3 +1,31 @@
+2010-03-09  Tony Chang  <tony@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        https://bugs.webkit.org/show_bug.cgi?id=21840
+        https://bugs.webkit.org/show_bug.cgi?id=23993
+
+        Fix an editing bug where replacing a selection would result in the
+        new text ending up inside nodes that were not visibly included in the
+        selection.  Instead, move our destination position out of nodes that
+        were not visibly included.
+
+        Two new tests to verify the new behavior.  Because we're now inserting
+        outside of some formatting nodes, some span tags are no longer necessary
+        for undoing formatting caused by these formatting nodes.
+
+        * editing/deleting/backspace-avoid-preceding-style-expected.txt: Added.
+        * editing/deleting/backspace-avoid-preceding-style.html: Added.
+        * editing/inserting/replace-at-visible-boundary-expected.txt: Added.
+        * editing/inserting/replace-at-visible-boundary.html: Added.
+        * platform/mac/editing/deleting/delete-3857753-fix-expected.txt:
+        * platform/mac/editing/inserting/insert-div-026-expected.txt:
+        * platform/mac/editing/pasteboard/paste-text-at-tabspan-001-expected.txt:
+        * platform/mac/editing/pasteboard/paste-text-at-tabspan-002-expected.txt:
+        * platform/mac/editing/style/font-family-with-space-expected.txt:
+        * platform/mac/editing/style/smoosh-styles-001-expected.txt:
+        * platform/mac/editing/style/style-boundary-005-expected.txt:
+
 2010-03-09  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         [Qt] http/tests/plugins/third-party-cookie-accept-policy.html was introduced in r55738,
diff --git a/LayoutTests/editing/deleting/backspace-avoid-preceding-style-expected.txt b/LayoutTests/editing/deleting/backspace-avoid-preceding-style-expected.txt
new file mode 100644 (file)
index 0000000..143fc6b
--- /dev/null
@@ -0,0 +1,2 @@
+This should be underlined.This should not be underlined.
+PASS
diff --git a/LayoutTests/editing/deleting/backspace-avoid-preceding-style.html b/LayoutTests/editing/deleting/backspace-avoid-preceding-style.html
new file mode 100644 (file)
index 0000000..285f6b2
--- /dev/null
@@ -0,0 +1,24 @@
+<body contentEditable="true">
+<div id="test-case">
+  <u>This should be underlined.</u><div id="not-underlined">This should not be underlined.</div>
+</div>
+<div id="results"></div>
+</body>
+<script src="../editing.js"></script>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    // Pressing delete to merge the two lines should not cause the text
+    // "not underlined" to be inside the <u> tag.
+    var secondLine = document.getElementById("not-underlined");
+    execSetSelectionCommand(secondLine, 0, secondLine, 0);
+    document.execCommand("Delete");
+
+    var result = document.getElementById("test-case").innerHTML;
+    result = result.replace(/^\s+/g, "").replace(/\s+$/g, "");  // Removing leading/trailing whitespace.
+    if (result == "<u>This should be underlined.</u>This should not be underlined.")
+        document.getElementById("results").innerText = "PASS";
+    else
+        document.getElementById("results").innerText = "FAIL";
+</script>
diff --git a/LayoutTests/editing/inserting/replace-at-visible-boundary-expected.txt b/LayoutTests/editing/inserting/replace-at-visible-boundary-expected.txt
new file mode 100644 (file)
index 0000000..ff43ca4
--- /dev/null
@@ -0,0 +1 @@
+SUCCESS
diff --git a/LayoutTests/editing/inserting/replace-at-visible-boundary.html b/LayoutTests/editing/inserting/replace-at-visible-boundary.html
new file mode 100644 (file)
index 0000000..6d83a63
--- /dev/null
@@ -0,0 +1,36 @@
+<body contentEditable="true">
+<b id="bold">bold</b>regular text<br>
+</body>
+<script src="../editing.js"></script>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    function fail(msg) {
+        console.log(msg);
+        throw msg;
+    }
+
+    // Inserting HTML over "regular text" shouldn't be in the bold tag.
+    var bold = document.getElementById("bold");
+    var unboldText = bold.nextSibling;
+    execSetSelectionCommand(unboldText, 0, unboldText, unboldText.textContent.length);
+    document.execCommand("insertHTML", false, "<img id='img' src='../resources/abe.png' /> not bold");
+
+    // Verify that the image isn't in the bold tag.
+    var image = document.getElementById("img");
+    if (image.previousSibling != bold)
+        fail("Image should be adjacent to the bold node.");
+
+    // Now try inserting HTML over the image.
+    execSetSelectionCommand(image, 0, image, 1);
+    document.execCommand("inserthtml", false, "<span id='red' style='color:red'>red text</span>");
+
+    // Verify that the red text isn't in the bold tag.
+    var red = document.getElementById("red");
+    if (red.previousSibling != bold)
+        fail("Red text should be adjacent to the bold node.");
+
+    // Replace text with SUCCESS.
+    document.body.innerHTML = "SUCCESS";
+</script>
index 3540bbe..564e381 100644 (file)
@@ -18,13 +18,12 @@ 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
-        RenderInline {B} at (0,0) size 37x28
+        RenderInline {B} at (0,0) size 25x28
           RenderText {#text} at (0,0) size 25x28
             text run at (0,0) width 25: "on"
-          RenderInline {SPAN} at (0,0) size 12x28
-            RenderInline {I} at (0,0) size 12x28
-              RenderText {#text} at (25,0) size 12x28
-                text run at (25,0) width 12: "o"
+        RenderInline {I} at (0,0) size 12x28
+          RenderText {#text} at (25,0) size 12x28
+            text run at (25,0) width 12: "o"
       RenderBlock {DIV} at (14,42) size 756x28
         RenderInline {B} at (0,0) size 54x28
           RenderText {#text} at (0,0) size 54x28
index 6c3d375..1574d73 100644 (file)
@@ -52,10 +52,9 @@ layer at (0,0) size 800x600
             text run at (462,28) width 12: "x"
       RenderBlock {DIV} at (0,236) size 784x32
         RenderBlock {DIV} at (0,0) size 784x32 [border: (2px solid #FF0000)]
-          RenderInline {B} at (0,0) size 32x28
+          RenderInline {B} at (0,0) size 20x28
             RenderText {#text} at (2,2) size 20x28
               text run at (2,2) width 20: "fo"
-            RenderInline {SPAN} at (0,0) size 12x28
-              RenderText {#text} at (22,2) size 12x28
-                text run at (22,2) width 12: "x"
+          RenderText {#text} at (22,2) size 12x28
+            text run at (22,2) width 12: "x"
 caret: position 3 of child 0 {#text} of child 0 {B} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 66db667..30240c5 100644 (file)
@@ -4,10 +4,10 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -19,12 +19,13 @@ layer at (0,0) size 800x600
         RenderInline {SPAN} at (0,0) size 155x28
           RenderText {#text} at (14,14) size 11x28
             text run at (14,14) width 11: "a"
-          RenderText {#text} at (25,14) size 23x28
-            text run at (25,14) width 23: "ax"
-          RenderInline {SPAN} at (0,0) size 110x28
+          RenderInline {SPAN} at (0,0) size 133x28
+            RenderInline {SPAN} at (0,0) size 23x28
+              RenderText {#text} at (25,14) size 23x28
+                text run at (25,14) width 23: "ax"
             RenderText {#text} at (48,14) size 110x28
               text run at (48,14) width 110: "\x{9}\x{9}\x{9}"
           RenderText {#text} at (158,14) size 11x28
             text run at (158,14) width 11: "z"
         RenderText {#text} at (0,0) size 0x0
-caret: position 2 of child 1 {#text} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 2 of child 0 {#text} of child 0 {SPAN} of child 1 {SPAN} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 22b1825..bc7aa61 100644 (file)
@@ -5,10 +5,10 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -23,12 +23,13 @@ layer at (0,0) size 800x600
           RenderInline {SPAN} at (0,0) size 37x28
             RenderText {#text} at (25,14) size 37x28
               text run at (25,14) width 37: "\x{9}"
-          RenderText {#text} at (62,14) size 23x28
-            text run at (62,14) width 23: "ax"
-          RenderInline {SPAN} at (0,0) size 73x28
+          RenderInline {SPAN} at (0,0) size 96x28
+            RenderInline {SPAN} at (0,0) size 23x28
+              RenderText {#text} at (62,14) size 23x28
+                text run at (62,14) width 23: "ax"
             RenderText {#text} at (85,14) size 73x28
               text run at (85,14) width 73: "\x{9}\x{9}"
           RenderText {#text} at (158,14) size 11x28
             text run at (158,14) width 11: "z"
         RenderText {#text} at (0,0) size 0x0
-caret: position 2 of child 2 {#text} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 2 of child 0 {#text} of child 0 {SPAN} of child 2 {SPAN} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 2b128c0..09961f0 100644 (file)
@@ -3,13 +3,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
-      RenderInline {U} at (0,0) size 490x15
+      RenderInline {U} at (0,0) size 245x15
         RenderText {#text} at (0,0) size 245x15
           text run at (0,0) width 245: "This text should be Times New Roman bold."
-        RenderInline {SPAN} at (0,0) size 245x15
-          RenderInline {U} at (0,0) size 245x15
-            RenderText {#text} at (245,0) size 245x15
-              text run at (245,0) width 245: "This text should be Times New Roman bold."
+      RenderInline {U} at (0,0) size 245x15
+        RenderText {#text} at (245,0) size 245x15
+          text run at (245,0) width 245: "This text should be Times New Roman bold."
       RenderText {#text} at (0,0) size 0x0
       RenderText {#text} at (0,0) size 0x0
-caret: position 41 of child 0 {#text} of child 0 {U} of child 1 {SPAN} of child 0 {U} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 41 of child 0 {#text} of child 1 {U} of child 1 {BODY} of child 0 {HTML} of document
index c66ab4e..ff8e837 100644 (file)
@@ -18,7 +18,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > SPAN > SPAN > DIV > DIV > BODY > HTML > #document to 3 of #text > SPAN > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > DIV > DIV > BODY > HTML > #document to 3 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -51,14 +51,13 @@ layer at (0,0) size 800x600
           RenderText {#text} at (0,0) size 0x0
       RenderBlock {DIV} at (0,208) size 784x32
         RenderBlock {DIV} at (0,0) size 784x32 [border: (2px solid #FF0000)]
-          RenderInline {SPAN} at (0,0) size 77x28 [color=#FF0000]
+          RenderInline {SPAN} at (0,0) size 23x28 [color=#FF0000]
             RenderText {#text} at (2,2) size 23x28
               text run at (2,2) width 23: "ab"
-            RenderInline {SPAN} at (0,0) size 54x28 [color=#000000]
-              RenderText {#text} at (25,2) size 34x28
-                text run at (25,2) width 34: "cde"
-              RenderInline {SPAN} at (0,0) size 20x28 [color=#FF0000]
-                RenderText {#text} at (59,2) size 20x28
-                  text run at (59,2) width 20: "fg"
+          RenderText {#text} at (25,2) size 34x28
+            text run at (25,2) width 34: "cde"
+          RenderInline {SPAN} at (0,0) size 20x28 [color=#FF0000]
+            RenderText {#text} at (59,2) size 20x28
+              text run at (59,2) width 20: "fg"
         RenderBlock (anonymous) at (0,32) size 784x0
-caret: position 3 of child 0 {#text} of child 1 {SPAN} of child 1 {SPAN} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 3 of child 2 {#text} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index a2e123c..aee8ae9 100644 (file)
@@ -24,7 +24,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 5 of #text > B > DIV > DIV > BODY > HTML > #document to 5 of #text > B > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > B > DIV > DIV > BODY > HTML > #document to 5 of #text > B > DIV > DIV > BODY > HTML > #document toDOMRange:range from 4 of #text > SPAN > B > DIV > DIV > BODY > HTML > #document to 4 of #text > SPAN > B > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > B > DIV > DIV > BODY > HTML > #document to 5 of #text > B > DIV > DIV > BODY > HTML > #document toDOMRange:range from 4 of #text > DIV > DIV > BODY > HTML > #document to 4 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -59,12 +59,11 @@ layer at (0,0) size 800x600
         RenderBlock {DIV} at (0,0) size 784x22 [border: (2px solid #FF0000)]
           RenderText {#text} at (2,2) size 86x18
             text run at (2,2) width 86: "one two three"
-          RenderInline {B} at (0,0) size 60x18
+          RenderInline {B} at (0,0) size 33x18
             RenderText {#text} at (88,2) size 33x18
               text run at (88,2) width 33: " four"
-            RenderInline {SPAN} at (0,0) size 27x18
-              RenderText {#text} at (121,2) size 27x18
-                text run at (121,2) width 27: " one"
+          RenderText {#text} at (121,2) size 27x18
+            text run at (121,2) width 27: " one"
         RenderBlock (anonymous) at (0,22) size 784x0
           RenderText {#text} at (0,0) size 0x0
-caret: position 4 of child 0 {#text} of child 1 {SPAN} of child 1 {B} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 4 of child 2 {#text} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 556fff9..2b8d75f 100644 (file)
@@ -1,3 +1,22 @@
+2010-03-09  Tony Chang  <tony@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        https://bugs.webkit.org/show_bug.cgi?id=21840
+        https://bugs.webkit.org/show_bug.cgi?id=23993
+
+        Fix an editing bug where replacing a selection would result in the
+        new text ending up inside nodes that were not visibly included in the
+        selection.  Instead, move our destination position out of nodes that
+        were not visibly included.
+
+        Tests: editing/deleting/backspace-avoid-preceding-style.html
+               editing/inserting/replace-at-visible-boundary.html
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::positionAvoidingPrecedingNodes):
+        (WebCore::ReplaceSelectionCommand::doApply):
+
 2010-03-09  Brady Eidson  <beidson@apple.com>
 
         Reviewed by Tim Hatcher.
index 0e5696e..e4acf85 100644 (file)
@@ -104,6 +104,22 @@ static bool isInterchangeConvertedSpaceSpan(const Node *node)
            static_cast<const HTMLElement *>(node)->getAttribute(classAttr) == convertedSpaceSpanClassString;
 }
 
+static Position positionAvoidingPrecedingNodes(Position pos)
+{
+    // If we're already on a break, it's probably a placeholder and we shouldn't change our position.
+    if (pos.node()->hasTagName(brTag))
+        return pos;
+
+    // We also stop when changing block flow elements because even though the visual position is the
+    // same.  E.g.,
+    //   <div>foo^</div>^
+    // The two positions above are the same visual position, but we want to stay in the same block.
+    Node* stopNode = pos.node()->enclosingBlockFlowElement();
+    while (stopNode != pos.node() && VisiblePosition(pos) == VisiblePosition(pos.next()))
+        pos = pos.next();
+    return pos;
+}
+
 ReplacementFragment::ReplacementFragment(Document* document, DocumentFragment* fragment, bool matchStyle, const VisibleSelection& selection)
     : m_document(document),
       m_fragment(fragment),
@@ -885,7 +901,12 @@ void ReplaceSelectionCommand::doApply()
         frame->clearTypingStyle();
     
     bool handledStyleSpans = handleStyleSpansBeforeInsertion(fragment, insertionPos);
-    
+
+    // We don't want the destination to end up inside nodes that weren't selected.  To avoid that, we move the
+    // position forward without changing the visible position so we're still at the same visible location, but
+    // outside of preceding tags.
+    insertionPos = positionAvoidingPrecedingNodes(insertionPos);
+
     // FIXME: When pasting rich content we're often prevented from heading down the fast path by style spans.  Try
     // again here if they've been removed.