LayoutTests:

        Reviewed by darin

        <rdar://problem/5134759>
        GMail Editor: Hang after pasting underlined text multiple times

        Demonstrates the bug:
        * editing/pasteboard/5134759-expected.checksum: Added.
        * editing/pasteboard/5134759-expected.png: Added.
        * editing/pasteboard/5134759-expected.txt: Added.
        * editing/pasteboard/5134759.html: Added.

        A unnecessary extra space was added to an element's
        inline style declaration, because of the way cssText behaves:
        * editing/pasteboard/paste-table-002-expected.txt:

WebCore:

        Reviewed by darin

        <rdar://problem/5134759>
        GMail Editor: Hang after pasting underlined text multiple times

        The moveParagraphs call that ReplaceSelectionCommand
        performs must receive only inline content from createMarkup,
        or else it will result in another call to moveParagraphs
        when it performs the move, resulting in infinite recursion.

        * editing/markup.cpp:
        (WebCore::startMarkup): We were only converting a block to
        an inline if it had an inline style declaration or styles
        coming from matched rules.  Cleaned up this code a bit by
        handling an element's style separately from its other
        attributes.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 7aa3d643292c7a1f23324ff884b3e2a5afcfad18..d09cdab9faa3e08dc9f5a7d073db591552e488d4 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,20 @@
+2007-04-16  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <rdar://problem/5134759> 
+        GMail Editor: Hang after pasting underlined text multiple times
+
+        Demonstrates the bug:
+        * editing/pasteboard/5134759-expected.checksum: Added.
+        * editing/pasteboard/5134759-expected.png: Added.
+        * editing/pasteboard/5134759-expected.txt: Added.
+        * editing/pasteboard/5134759.html: Added.
+        
+        A unnecessary extra space was added to an element's 
+        inline style declaration, because of the way cssText behaves:
+        * editing/pasteboard/paste-table-002-expected.txt:
+
 2007-04-13  David Kilzer  <ddkilzer@webkit.org>
 
         Patch and review by hyatt.  Testing and landing by ddkilzer.

diff --git a/LayoutTests/editing/pasteboard/5134759-expected.checksum b/LayoutTests/editing/pasteboard/5134759-expected.checksum
new file mode 100644 (file)
index 0000000..b28eb4d
--- /dev/null
+++ b/LayoutTests/editing/pasteboard/5134759-expected.checksum
@@ -0,0 +1 @@
+2bbca86f137f72d6dbeabd3ddd670f74
\ No newline at end of file

diff --git a/LayoutTests/editing/pasteboard/5134759-expected.png b/LayoutTests/editing/pasteboard/5134759-expected.png
new file mode 100644 (file)
index 0000000..c0cb209
Binary files /dev/null and b/LayoutTests/editing/pasteboard/5134759-expected.png differ

diff --git a/LayoutTests/editing/pasteboard/5134759-expected.txt b/LayoutTests/editing/pasteboard/5134759-expected.txt
new file mode 100644 (file)
index 0000000..1199b08
--- /dev/null
+++ b/LayoutTests/editing/pasteboard/5134759-expected.txt
@@ -0,0 +1,24 @@
+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 784x18
+        RenderText {#text} at (0,0) size 648x18
+          text run at (0,0) width 504: "This tests for a hang when pasting underlined content at the end of a paragraph. "
+          text run at (504,0) width 144: "You should see 'Hello "
+        RenderInline {U} at (0,0) size 45x18
+          RenderText {#text} at (648,0) size 45x18
+            text run at (648,0) width 45: "World!"
+        RenderText {#text} at (693,0) size 50x18
+          text run at (693,0) width 50: "' below."
+      RenderBlock {DIV} at (0,34) size 784x18
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {#text} at (0,0) size 39x18
+            text run at (0,0) width 39: "Hello "
+          RenderInline {SPAN} at (0,0) size 45x18
+            RenderInline {DIV} at (0,0) size 45x18
+              RenderText {#text} at (39,0) size 45x18
+                text run at (39,0) width 45: "World!"
+        RenderBlock (anonymous) at (0,18) size 784x0
+caret: position 6 of child 0 {#text} of child 0 {DIV} of child 1 {SPAN} of child 0 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document

diff --git a/LayoutTests/editing/pasteboard/5134759.html b/LayoutTests/editing/pasteboard/5134759.html
new file mode 100644 (file)
index 0000000..22c985f
--- /dev/null
+++ b/LayoutTests/editing/pasteboard/5134759.html
@@ -0,0 +1,10 @@
+<p>This tests for a hang when pasting underlined content at the end of a paragraph.  You should see 'Hello <u>World!</u>' below.</p>
+
+<div id="div" contenteditable="true"><div>Hello&nbsp;</div></div>
+
+<script>
+var sel = window.getSelection();
+var div = document.getElementById("div");
+sel.setPosition(div, div.childNodes.length);
+document.execCommand("InsertHTML", false, "<span style='text-decoration: underline;'><div>World!</div></span>");
+</script>

diff --git a/LayoutTests/editing/pasteboard/paste-table-002-expected.txt b/LayoutTests/editing/pasteboard/paste-table-002-expected.txt
index 953fbeaa8afa4f6c97d4e0bbd731dca9629c0508..2191dc5cd9eb71dcd1e9b08c7017e0998fa0718d 100644 (file)
--- a/LayoutTests/editing/pasteboard/paste-table-002-expected.txt
+++ b/LayoutTests/editing/pasteboard/paste-table-002-expected.txt
@@ -16,5 +16,5 @@ Problem: copy/pasting some HTML including tables can give rise to a <div> elemen
 abcdef
 foo    bar
 ghijk
-<div id="test" class="editing"> <div>abcdef<div style="text-align: center"><table><tbody><tr><td>foo</td><td>bar</td></tr></tbody></table>ghijk</div> </div> </div>
+<div id="test" class="editing"> <div>abcdef<div style="text-align: center; "><table><tbody><tr><td>foo</td><td>bar</td></tr></tbody></table>ghijk</div> </div> </div>
 

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index eb57a5c726a8a4ef2a967fa1efb1e526bbdc22a3..40717cc13540c77a492cd8b21c9fd51cdfc53dfc 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2007-04-16  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        <rdar://problem/5134759> 
+        GMail Editor: Hang after pasting underlined text multiple times
+        
+        The moveParagraphs call that ReplaceSelectionCommand
+        performs must receive only inline content from createMarkup, 
+        or else it will result in another call to moveParagraphs 
+        when it performs the move, resulting in infinite recursion.
+
+        * editing/markup.cpp:
+        (WebCore::startMarkup): We were only converting a block to
+        an inline if it had an inline style declaration or styles 
+        coming from matched rules.  Cleaned up this code a bit by 
+        handling an element's style separately from its other 
+        attributes.
+
 2007-04-16  Darin Adler  <darin@apple.com>
 
         - get layout tests going again

diff --git a/WebCore/editing/markup.cpp b/WebCore/editing/markup.cpp
index 0bd0c35743c9f514fd6c89ab5246eeda1c8a4e30..9f1a26f849bfcd88c4af43529731ff3dec73d6bb 100644 (file)
--- a/WebCore/editing/markup.cpp
+++ b/WebCore/editing/markup.cpp
@@ -191,30 +191,15 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
             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();
-            }
             NamedAttrMap *attrs = el->attributes();
             unsigned length = attrs->length();
 
             for (unsigned int i = 0; i < length; i++) {
                 Attribute *attr = attrs->attributeItem(i);
+                // We'll handle the style attribute separately, below.
+                if (attr->name() == styleAttr && el->isHTMLElement() && (annotate || convertBlocksToInlines))
+                    continue;
                 String value = attr->value();
-                if (attr->name() == styleAttr && convertBlocksToInlines && el->isHTMLElement()) {
-                    Element* element = const_cast<Element*>(el);
-                    RefPtr<CSSMutableStyleDeclaration> inlineStyle = static_cast<HTMLElement*>(element)->getInlineStyleDecl()->copy();
-                    inlineStyle->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE, true);
-                    value = inlineStyle->cssText();
-                }
-                if (annotate && attr->name() == styleAttr && additionalStyle.length()) {
-                    value += "; " + additionalStyle;
-                    additionalStyle = "";
-                }
                 // FIXME: Handle case where value has illegal characters in it, like "
                 if (documentIsHTML)
                     markup += " " + attr->name().localName().deprecatedString();
@@ -223,9 +208,18 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
                 markup += "=\"" + escapeTextForMarkup(value.deprecatedString(), true) + "\"";
             }
             
-            if (annotate && additionalStyle.length())
-                // FIXME: Handle case where additionalStyle has illegal characters in it, like "
-                markup += " " +  styleAttr.localName().deprecatedString() + "=\"" + additionalStyle.deprecatedString() + "\"";
+            if (el->isHTMLElement() && (annotate || convertBlocksToInlines)) {
+                Element* element = const_cast<Element*>(el);
+                RefPtr<CSSMutableStyleDeclaration> style = static_cast<HTMLElement*>(element)->getInlineStyleDecl()->copy();
+                if (annotate) {
+                    RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(const_cast<Element*>(el));
+                    style->merge(styleFromMatchedRules.get());
+                }
+                if (convertBlocksToInlines)
+                    style->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE, true);
+                if (style->length() > 0)
+                    markup += " " +  styleAttr.localName().deprecatedString() + "=\"" + style->cssText().deprecatedString() + "\"";
+            }
             
             if (shouldSelfClose(el)) {
                 if (el->isHTMLElement())