LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Jul 2006 19:49:10 +0000 (19:49 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Jul 2006 19:49:10 +0000 (19:49 +0000)
        Reviewed by justin

        <http://bugzilla.opendarwin.org/show_bug.cgi?id=9507>
        Empty style spans created in applyInlineStyle
        <rdar://problem/4515463>
        REGRESSION: Blot and Mail both do a very poor job of pasting the main www.apple.com page

        * editing/pasteboard/paste-4039777-fix-expected.txt:
        * editing/pasteboard/testcase-9507-expected.checksum: Added.
        * editing/pasteboard/testcase-9507-expected.png: Added.
        * editing/pasteboard/testcase-9507-expected.txt: Added.
        * editing/pasteboard/testcase-9507.html: Added.
        * editing/unsupported-content/table-delete-001-expected.txt:
        * editing/selection/selectNode-expected.png:
        * editing/selection/selectNode-expected.checksum:
        * editing/selection/selectNode-expected.txt:

WebCore:

        Reviewed by justin

        <http://bugzilla.opendarwin.org/show_bug.cgi?id=9507>
        Empty style spans created in applyInlineStyle

        Improves paste fidelity because some of these empty font/style spans had a non-zero
        size and were messing up the layout of pasted content:
        <rdar://problem/4515463>
        REGRESSION: Blot and Mail both do a very poor job of pasting the main www.apple.com page

        * editing/ApplyStyleCommand.cpp:
        (WebCore::ApplyStyleCommand::applyInlineStyle): Use the adjusted start node instead
        of start.node().  Don't do any application if the endpoints are swapped.  Adjust
        endNode if the start node is a descendant of it, so that the pre-order traversal will
        terminate properly.

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

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/paste-4039777-fix-expected.txt
LayoutTests/editing/pasteboard/testcase-9507-expected.checksum [new file with mode: 0644]
LayoutTests/editing/pasteboard/testcase-9507-expected.png [new file with mode: 0644]
LayoutTests/editing/pasteboard/testcase-9507-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/testcase-9507.html [new file with mode: 0644]
LayoutTests/editing/unsupported-content/table-delete-001-expected.txt
WebCore/ChangeLog
WebCore/editing/ApplyStyleCommand.cpp

index 7b8c8dd..362c6e8 100644 (file)
@@ -1,3 +1,22 @@
+2006-07-31  Graham Dennis  <graham.dennis@gmail.com>
+
+        Reviewed by justin
+        
+        <http://bugzilla.opendarwin.org/show_bug.cgi?id=9507>
+        Empty style spans created in applyInlineStyle
+        <rdar://problem/4515463>
+        REGRESSION: Blot and Mail both do a very poor job of pasting the main www.apple.com page        
+
+        * editing/pasteboard/paste-4039777-fix-expected.txt:
+        * editing/pasteboard/testcase-9507-expected.checksum: Added.
+        * editing/pasteboard/testcase-9507-expected.png: Added.
+        * editing/pasteboard/testcase-9507-expected.txt: Added.
+        * editing/pasteboard/testcase-9507.html: Added.
+        * editing/unsupported-content/table-delete-001-expected.txt:
+        * editing/selection/selectNode-expected.png:
+        * editing/selection/selectNode-expected.checksum:
+        * editing/selection/selectNode-expected.txt:
+
 2006-07-31  Mark Rowe  <opendarwin.org@bdash.net.nz>
 
         Reviewed by Darin.
@@ -13,7 +32,7 @@
         affected by my recent change to getComputedStyle.
 
         * editing/pasteboard/paste-table-002.html: Converted to a text test, since
-        it dumps the serialized form of hte result.
+        it dumps the serialized form of the result.
         * editing/pasteboard/paste-table-002-expected.checksum: Removed.
         * editing/pasteboard/paste-table-002-expected.png: Removed.
 
index 44a55f9..764bf07 100644 (file)
@@ -52,9 +52,6 @@ layer at (0,0) size 800x600
                   text run at (0,28) width 16: "C"
       RenderBlock {DIV} at (0,364) size 784x148
         RenderBlock {UL} at (0,0) size 784x28
-          RenderBlock (anonymous) at (40,0) size 744x0
-            RenderInline {FONT} at (0,0) size 0x0
-            RenderInline {SPAN} at (0,0) size 0x0
           RenderListItem {LI} at (40,0) size 744x28
             RenderListMarker at (-17,8) size 7x18
             RenderInline {FONT} at (0,0) size 17x28
@@ -62,13 +59,7 @@ layer at (0,0) size 800x600
                 RenderText {#text} at (727,0) size 17x28
                   text run at (727,0) width 17: "A"
         RenderBlock {DIV} at (0,44) size 784x56
-          RenderBlock (anonymous) at (0,0) size 784x0
-            RenderInline {FONT} at (0,0) size 0x0
-            RenderInline {SPAN} at (0,0) size 0x0
           RenderBlock {UL} at (0,0) size 784x56
-            RenderBlock (anonymous) at (40,0) size 744x0
-              RenderInline {FONT} at (0,0) size 0x0
-              RenderInline {SPAN} at (0,0) size 0x0
             RenderListItem {LI} at (40,0) size 744x56
               RenderListMarker at (-17,8) size 7x18
               RenderInline {A} at (0,0) size 16x18 [color=#0000EE]
@@ -76,19 +67,15 @@ layer at (0,0) size 800x600
                   RenderInline {SPAN} at (0,0) size 16x28
                     RenderText {#text} at (0,0) size 16x28
                       text run at (0,0) width 16: "B"
-              RenderInline {FONT} at (0,0) size 6x28
-                RenderInline {SPAN} at (0,0) size 6x28
+              RenderInline {FONT} at (0,0) size 22x56
+                RenderInline {SPAN} at (0,0) size 22x56
                   RenderText {#text} at (16,0) size 6x28
                     text run at (16,0) width 6: " "
-              RenderInline {FONT} at (0,0) size 0x28
-                RenderInline {SPAN} at (0,0) size 0x28
                   RenderBR {BR} at (22,22) size 0x0
-              RenderInline {FONT} at (0,0) size 16x28
-                RenderInline {SPAN} at (0,0) size 16x28
                   RenderText {#text} at (0,28) size 16x28
                     text run at (0,28) width 16: "C"
         RenderBlock {DIV} at (0,116) size 784x32 [border: (2px solid #FF0000)]
           RenderBlock (anonymous) at (2,2) size 780x28
             RenderBR {BR} at (780,0) size 0x28
           RenderBlock {DIV} at (2,30) size 780x0
-caret: position 1 of child 0 {#text} of child 0 {SPAN} of child 3 {FONT} of child 2 {LI} of child 2 {UL} of child 2 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 1 of child 2 {#text} of child 0 {SPAN} of child 1 {FONT} of child 0 {LI} of child 0 {UL} of child 2 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/pasteboard/testcase-9507-expected.checksum b/LayoutTests/editing/pasteboard/testcase-9507-expected.checksum
new file mode 100644 (file)
index 0000000..a89330b
--- /dev/null
@@ -0,0 +1 @@
+bf727e283e2b96309e191187cfcae865
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/testcase-9507-expected.png b/LayoutTests/editing/pasteboard/testcase-9507-expected.png
new file mode 100644 (file)
index 0000000..fd0d642
Binary files /dev/null and b/LayoutTests/editing/pasteboard/testcase-9507-expected.png differ
diff --git a/LayoutTests/editing/pasteboard/testcase-9507-expected.txt b/LayoutTests/editing/pasteboard/testcase-9507-expected.txt
new file mode 100644 (file)
index 0000000..aedadd9
--- /dev/null
@@ -0,0 +1,29 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 3 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 1 of #text > DIV > DIV > BODY > HTML > #document to 3 of #text > DIV > DIV > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > FONT > DIV > DIV > DIV > DIV > BODY > HTML > #document to 3 of #text > FONT > DIV > DIV > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+layer at (0,0) size 800x600
+  RenderView 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 784x36
+        RenderText {#text} at (0,0) size 780x36
+          text run at (0,0) width 780: "When copying some text, under certain circumstances, empty style (or font) tags are created. This test checks that there is no"
+          text run at (0,18) width 277: "empty <font> tag after 'foo' and before 'bar'."
+      RenderBlock {DIV} at (0,36) size 784x60
+        RenderBlock {DIV} at (0,0) size 784x60 [border: (2px solid #FF0000)]
+          RenderBlock (anonymous) 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
+            RenderBlock {DIV} at (0,0) size 780x28
+              RenderInline {FONT} at (0,0) size 31x28 [color=#FF0000]
+                RenderText {#text} at (0,0) size 31x28
+                  text run at (0,0) width 31: "bar"
+          RenderBlock (anonymous) at (2,58) size 780x0
+          RenderBlock {DIV} at (2,58) size 780x0 [color=#FF0000]
+caret: position 3 of child 0 {#text} of child 0 {FONT} of child 0 {DIV} of child 1 {DIV} of child 1 {DIV} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/pasteboard/testcase-9507.html b/LayoutTests/editing/pasteboard/testcase-9507.html
new file mode 100644 (file)
index 0000000..7dad924
--- /dev/null
@@ -0,0 +1,48 @@
+<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() {
+    setSelectionCommand(document.getElementById("test"),0,document.getElementById("test"),5);
+    copyCommand();
+    pasteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+When copying some text, under certain circumstances, empty style (or font) tags are created. This test checks that there is no empty &lt;font&gt; tag after 'foo' and before 'bar'.
+<div contenteditable id="root">
+<div id="test" class="editing">
+foo
+<div style="color: rgb(255, 0, 0);" >
+<div>bar</div>
+</div>
+</div>
+</div>
+
+<script>
+editingTest();
+</script>
+
+</body>
+</html>
index 5bcb698..b0e7dc2 100644 (file)
@@ -21,7 +21,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 0 of #text > TD > TR > TBODY > TABLE > DIV > DIV > BODY > HTML > #document to 2 of TABLE > DIV > DIV > BODY > HTML > #document
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of SPAN > FONT > DIV > DIV > BODY > HTML > #document to 1 of SPAN > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 2 of DIV > DIV > BODY > HTML > #document to 2 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -65,9 +65,7 @@ layer at (0,0) size 800x600
         RenderBlock {DIV} at (0,0) size 784x60 [border: (2px solid #FF0000)]
           RenderText {#text} at (2,2) size 62x28
             text run at (2,2) width 62: "before"
-          RenderInline {FONT} at (0,0) size 0x22
-            RenderInline {SPAN} at (0,0) size 0x18
-              RenderBR {BR} at (64,24) size 0x0
+          RenderBR {BR} at (64,24) size 0x0
           RenderText {#text} at (2,30) size 45x28
             text run at (2,30) width 45: "after"
 caret: position 0 of child 2 {#text} of child 1 {DIV} of child 5 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 24ee4e8..8e131fe 100644 (file)
@@ -1,3 +1,21 @@
+2006-07-31  Graham Dennis  <graham.dennis@gmail.com>
+
+        Reviewed by justin
+        
+        <http://bugzilla.opendarwin.org/show_bug.cgi?id=9507>
+        Empty style spans created in applyInlineStyle
+        
+        Improves paste fidelity because some of these empty font/style spans had a non-zero
+        size and were messing up the layout of pasted content:
+        <rdar://problem/4515463>
+        REGRESSION: Blot and Mail both do a very poor job of pasting the main www.apple.com page   
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::applyInlineStyle): Use the adjusted start node instead 
+        of start.node().  Don't do any application if the endpoints are swapped.  Adjust
+        endNode if the start node is a descendant of it, so that the pre-order traversal will
+        terminate properly.
+
 2006-07-31  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Darin.
index 9e4fe49..bef2ce3 100644 (file)
@@ -585,16 +585,28 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclaration *style)
     
     Node* node = start.node();
     Node* endNode = end.node();
+    
+    bool rangeIsEmpty = false;
 
-    if (start.offset() >= start.node()->caretMaxOffset())
+    if (start.offset() >= start.node()->caretMaxOffset()) {
         node = node->traverseNextNode();
+        Position newStart = Position(node, 0);
+        if (Range::compareBoundaryPoints(end, newStart) < 0)
+            rangeIsEmpty = true;
+    }
 
     if (end.node()->renderer() && end.node()->renderer()->isTable() && end.offset() == maxDeepOffset(end.node()))
         endNode = end.node()->lastDescendant();
     
-    if (start.node() == endNode) {
+    if (node == endNode) {
         addInlineStyleIfNeeded(style, node, node);
-    } else {
+    } else if (!rangeIsEmpty) {
+        // If the end node is an ancestor of the start node then we need
+        // to replace endNode with its next sibling to ensure that the
+        // exit condition node == endNode is reached
+        if (node->isAncestor(endNode))
+            endNode = endNode->traverseNextSibling();
+    
         while (1) {
             if (node->childNodeCount() == 0 && node->renderer() && node->renderer()->isInline() && node->isContentRichlyEditable()) {
                 Node *runStart = node;