LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Mar 2007 22:55:03 +0000 (22:55 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Mar 2007 22:55:03 +0000 (22:55 +0000)
        Reviewed by darin

        <rdar://problem/5091898> REGRESSION: GMail Editor: A line of underlined text copied with Select All pastes with wrong font family

        Demonstrates the bug fixed:
        * editing/style/5091898-expected.checksum: Added.
        * editing/style/5091898-expected.png: Added.
        * editing/style/5091898-expected.txt: Added.
        * editing/style/5091898.html: Added.
        Demonstrates a related bug fixed (deleting
        the line break before a paragraph could remove
        its underlining):
        * editing/deleting/5091898-expected.checksum: Added.
        * editing/deleting/5091898-expected.png: Added.
        * editing/deleting/5091898-expected.txt: Added.
        * editing/deleting/5091898.html: Added.
        Demonstrates that a link now (correctly) exists,
        instead of just underlined text:
        * editing/pasteboard/4840662-expected.txt:

WebCore:

        Reviewed by darin

        <rdar://problem/5091898> REGRESSION: GMail Editor: A line of underlined text copied with Select All pastes with wrong font family

        createMarkup skipped elements if they were blocks
        when called from moveParagraphs (because that function
        must receive only inline content).  This patch adds
        code to inline these blocks instead of skipping them
        so that we don't lose any of the style that they
        contribute to the copied markup.

        * editing/markup.cpp:
        (WebCore::startMarkup): Add an inlineBlocks option.
        Make sure to overwrite display:block coming from
        a style sheet or the inline style declaration.
        (WebCore::createMarkup): Don't refuse to include a
        specialCommonAncestor that's a block if we were asked
        to include only inline content, since we can now inline
        block elements in startMarkup.

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/deleting/5091898-expected.checksum [new file with mode: 0644]
LayoutTests/editing/deleting/5091898-expected.png [new file with mode: 0644]
LayoutTests/editing/deleting/5091898-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/5091898.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/4840662-expected.txt
LayoutTests/editing/style/5091898-expected.checksum [new file with mode: 0644]
LayoutTests/editing/style/5091898-expected.png [new file with mode: 0644]
LayoutTests/editing/style/5091898-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/5091898.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/markup.cpp
WebCore/editing/markup.h

index 76f80e7ffdad70538d12a4133fd7d06e36fb6383..da721cfa3fcf9de5f81ebc449e058b62624a14d7 100644 (file)
@@ -1,3 +1,25 @@
+2007-03-27  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <rdar://problem/5091898> REGRESSION: GMail Editor: A line of underlined text copied with Select All pastes with wrong font family
+
+        Demonstrates the bug fixed:
+        * editing/style/5091898-expected.checksum: Added.
+        * editing/style/5091898-expected.png: Added.
+        * editing/style/5091898-expected.txt: Added.
+        * editing/style/5091898.html: Added.
+        Demonstrates a related bug fixed (deleting
+        the line break before a paragraph could remove
+        its underlining):
+        * editing/deleting/5091898-expected.checksum: Added.
+        * editing/deleting/5091898-expected.png: Added.
+        * editing/deleting/5091898-expected.txt: Added.
+        * editing/deleting/5091898.html: Added.
+        Demonstrates that a link now (correctly) exists, 
+        instead of just underlined text:
+        * editing/pasteboard/4840662-expected.txt:
+
 2007-03-26  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by darin
diff --git a/LayoutTests/editing/deleting/5091898-expected.checksum b/LayoutTests/editing/deleting/5091898-expected.checksum
new file mode 100644 (file)
index 0000000..45d7ab7
--- /dev/null
@@ -0,0 +1 @@
+dd8c13473ecb22f289f3387847af2827
\ No newline at end of file
diff --git a/LayoutTests/editing/deleting/5091898-expected.png b/LayoutTests/editing/deleting/5091898-expected.png
new file mode 100644 (file)
index 0000000..6ba712c
Binary files /dev/null and b/LayoutTests/editing/deleting/5091898-expected.png differ
diff --git a/LayoutTests/editing/deleting/5091898-expected.txt b/LayoutTests/editing/deleting/5091898-expected.txt
new file mode 100644 (file)
index 0000000..5c24e6f
--- /dev/null
@@ -0,0 +1,16 @@
+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 {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 767x36
+          text run at (0,0) width 767: "This tests for a bug where underlined content would lose its underlining when deleting the line break before the paragraph"
+          text run at (0,18) width 104: "that contained it."
+      RenderBlock {DIV} at (0,52) size 784x18
+        RenderText {#text} at (0,0) size 183x18
+          text run at (0,0) width 183: "This shouldn't be underlined."
+        RenderInline {DIV} at (0,0) size 168x18
+          RenderText {#text} at (183,0) size 168x18
+            text run at (183,0) width 168: "This should be underlined."
+caret: position 29 of child 0 {#text} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/deleting/5091898.html b/LayoutTests/editing/deleting/5091898.html
new file mode 100644 (file)
index 0000000..5992be3
--- /dev/null
@@ -0,0 +1,12 @@
+<p>This tests for a bug where underlined content would lose its underlining when deleting the line break before the paragraph that contained it.</p>
+<div contenteditable="true">This shouldn't be underlined.<div id="div" style="text-decoration: underline;">This should be underlined.</div></div>
+<script>
+var div = document.getElementById("div");
+var range = document.createRange();
+range.setStart(div, 0);
+range.setEnd(div, 0);
+var sel = window.getSelection();
+sel.addRange(range);
+document.execCommand("Delete");
+</script>
+
index 2b30e5d518f2fafd6ed385dc99de5b9a4d8987d7..0f9d7bc05648d3ea058779126abcc773a9aec7e2 100644 (file)
@@ -16,7 +16,7 @@ layer at (0,0) size 800x600
           RenderInline {SPAN} at (0,0) size 42x18 [color=#000000]
             RenderText {#text} at (21,0) size 20x18
               text run at (21,0) width 20: "bar"
-            RenderInline {SPAN} at (0,0) size 22x18 [color=#0000EE]
+            RenderInline {A} at (0,0) size 22x18 [color=#0000EE]
               RenderText {#text} at (41,0) size 22x18
                 text run at (41,0) width 22: "baz"
         RenderBlock (anonymous) at (0,18) size 784x0
diff --git a/LayoutTests/editing/style/5091898-expected.checksum b/LayoutTests/editing/style/5091898-expected.checksum
new file mode 100644 (file)
index 0000000..e0d5729
--- /dev/null
@@ -0,0 +1 @@
+e042889fa18fd7b6837a93c63fcd536c
\ No newline at end of file
diff --git a/LayoutTests/editing/style/5091898-expected.png b/LayoutTests/editing/style/5091898-expected.png
new file mode 100644 (file)
index 0000000..a83317f
Binary files /dev/null and b/LayoutTests/editing/style/5091898-expected.png differ
diff --git a/LayoutTests/editing/style/5091898-expected.txt b/LayoutTests/editing/style/5091898-expected.txt
new file mode 100644 (file)
index 0000000..03a04fe
--- /dev/null
@@ -0,0 +1,16 @@
+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
+      RenderInline {SPAN} at (0,0) size 188x15
+        RenderText {#text} at (0,0) size 188x15
+          text run at (0,0) width 188: "This text should be Arial bold."
+      RenderInline {SPAN} at (0,0) size 192x15
+        RenderInline {SPAN} at (0,0) size 192x15
+          RenderInline {DIV} at (0,0) size 192x15
+            RenderText {#text} at (188,0) size 192x15
+              text run at (188,0) width 192: " This text should be Arial bold."
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+caret: position 32 of child 0 {#text} of child 0 {DIV} of child 1 {SPAN} of child 0 {SPAN} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/style/5091898.html b/LayoutTests/editing/style/5091898.html
new file mode 100644 (file)
index 0000000..859b6ea
--- /dev/null
@@ -0,0 +1,11 @@
+<html style="font-family: Times New Roman; font-size: 13px;">
+<body contenteditable="true" style="font-family: Arial; font-weight: bold;">This text should be Arial bold.</body>
+<script>
+document.body.focus();
+document.execCommand("SelectAll");
+document.execCommand("Underline");
+document.execCommand("Copy");
+window.getSelection().modify("move", "forward", "character");
+document.execCommand("Paste");
+</script>
+</html>
index 43817cc6d44e2033a1b87564f3c646b429a7b026..8f93d652ea50cdc0d83c4835be1365dcd05c4e2a 100644 (file)
@@ -1,3 +1,25 @@
+2007-03-27  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <rdar://problem/5091898> REGRESSION: GMail Editor: A line of underlined text copied with Select All pastes with wrong font family
+        
+        createMarkup skipped elements if they were blocks
+        when called from moveParagraphs (because that function 
+        must receive only inline content).  This patch adds 
+        code to inline these blocks instead of skipping them 
+        so that we don't lose any of the style that they 
+        contribute to the copied markup.
+
+        * editing/markup.cpp:
+        (WebCore::startMarkup): Add an inlineBlocks option.
+        Make sure to overwrite display:block coming from
+        a style sheet or the inline style declaration.
+        (WebCore::createMarkup): Don't refuse to include a 
+        specialCommonAncestor that's a block if we were asked 
+        to include only inline content, since we can now inline 
+        block elements in startMarkup.
+
 2007-03-27  Adele Peterson  <adele@apple.com>
 
         Reviewed by Hyatt.
index e3be5c11b0111f34c1c101b9b45ba146cf2ea473..313829e5dfd99dec8158bfcab6c335a3bbe35b38 100644 (file)
@@ -154,7 +154,7 @@ static void removeEnclosingMailBlockquoteStyle(CSSMutableStyleDeclaration* style
     blockquoteStyle->diff(style);
 }
 
-static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnotateForInterchange annotate)
+static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnotateForInterchange annotate, bool convertBlocksToInlines = false)
 {
     bool documentIsHTML = node->document()->isHTMLDocument();
     switch (node->nodeType()) {
@@ -189,10 +189,13 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
         case Node::ELEMENT_NODE: {
             DeprecatedString markup = DeprecatedChar('<');
             const Element* el = static_cast<const Element*>(node);
+            convertBlocksToInlines &= isBlock(const_cast<Node*>(node));
             markup += el->nodeNamePreservingCase().deprecatedString();
             String additionalStyle;
             if (annotate && el->isHTMLElement()) {
                 RefPtr<CSSMutableStyleDeclaration> style = styleFromMatchedRulesForElement(const_cast<Element*>(el));
+                if (convertBlocksToInlines)
+                    style->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE, true);
                 if (style->length() > 0)
                     additionalStyle = style->cssText();
             }
@@ -202,6 +205,11 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
             for (unsigned int i = 0; i < length; i++) {
                 Attribute *attr = attrs->attributeItem(i);
                 String value = attr->value();
+                if (attr->name() == styleAttr && convertBlocksToInlines) {
+                    RefPtr<CSSMutableStyleDeclaration> inlineStyle = static_cast<const HTMLElement*>(el)->getInlineStyleDecl()->copy();
+                    inlineStyle->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE, true);
+                    value = inlineStyle->cssText();
+                }
                 if (annotate && attr->name() == styleAttr && additionalStyle.length()) {
                     value += "; " + additionalStyle;
                     additionalStyle = "";
@@ -351,7 +359,7 @@ bool elementHasTextDecorationProperty(Node* node)
 
 // FIXME: Shouldn't we omit style info when annotate == DoNotAnnotateForInterchange? 
 // FIXME: At least, annotation and style info should probably not be included in range.markupString()
-DeprecatedString createMarkup(const Range *range, Vector<Node*>* nodes, EAnnotateForInterchange annotate, bool includeInlineSpecialAncestors)
+DeprecatedString createMarkup(const Range *range, Vector<Node*>* nodes, EAnnotateForInterchange annotate, bool convertBlocksToInlines)
 {
     if (!range || range->isDetached())
         return DeprecatedString();
@@ -486,9 +494,6 @@ DeprecatedString createMarkup(const Range *range, Vector<Node*>* nodes, EAnnotat
         
     if (Node *enclosingAnchor = enclosingNodeWithTag(specialCommonAncestor ? specialCommonAncestor : commonAncestor, aTag))
         specialCommonAncestor = enclosingAnchor;
-        
-    if (!annotate && includeInlineSpecialAncestors && isBlock(specialCommonAncestor))
-        specialCommonAncestor = 0;
     
     Node* body = enclosingNodeWithTag(commonAncestor, bodyTag);
     // FIXME: Only include markup for a fully selected root (and ancestors of lastClosed up to that root) if
@@ -501,9 +506,7 @@ DeprecatedString createMarkup(const Range *range, Vector<Node*>* nodes, EAnnotat
     if (specialCommonAncestor) {
         // Also include all of the ancestors of lastClosed up to this special ancestor.
         for (Node* ancestor = lastClosed->parentNode(); ancestor; ancestor = ancestor->parentNode()) {
-            if (!annotate && includeInlineSpecialAncestors && isBlock(ancestor))
-                continue;
-            if (ancestor == fullySelectedRoot) {
+            if (ancestor == fullySelectedRoot && !convertBlocksToInlines) {
                 RefPtr<CSSMutableStyleDeclaration> style = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
                 
                 // Bring the background attribute over, but not as an attribute because a background attribute on a div
@@ -516,7 +519,7 @@ DeprecatedString createMarkup(const Range *range, Vector<Node*>* nodes, EAnnotat
                     markups.append("</div>");
                 }
             } else {
-                markups.prepend(startMarkup(ancestor, range, annotate));
+                markups.prepend(startMarkup(ancestor, range, annotate, convertBlocksToInlines));
                 markups.append(endMarkup(ancestor));
             }
             if (nodes)
index c5f5920375f88cb5967a88a8233dea59d99057f3..f0a349b5dcfface85852a4278cda67fdb0b2798d 100644 (file)
@@ -45,7 +45,7 @@ namespace WebCore {
     PassRefPtr<DocumentFragment> createFragmentFromNodes(Document*, const Vector<Node*>&);
 
     DeprecatedString createMarkup(const Range*,
-        Vector<Node*>* = 0, EAnnotateForInterchange = DoNotAnnotateForInterchange, bool includeInlineSpecialAncestors = false);
+        Vector<Node*>* = 0, EAnnotateForInterchange = DoNotAnnotateForInterchange, bool convertBlocksToInlines = false);
     DeprecatedString createMarkup(const Node*, EChildrenOnly = IncludeNode,
         Vector<Node*>* = 0, EAnnotateForInterchange = DoNotAnnotateForInterchange);