Commit 2509fab1 authored by tony@chromium.org's avatar tony@chromium.org

2010-02-17 Tony Chang <tony@chromium.org>

        Reviewed by Eric Seidel.

        https://bugs.webkit.org/show_bug.cgi?id=26937
        No longer allow span styles to surround block elements like
        divs when pasting (see paste-text-012 and 5065605 which had divs in
        spans).  This also causes a few cases of empty spans to be removed
        (see 19089 and 5245519).

        * editing/execCommand/19089-expected.txt:
        * editing/pasteboard/5245519-expected.txt:
        * editing/pasteboard/5245519.html:
        * platform/mac/editing/pasteboard/5065605-expected.txt:
        * platform/mac/editing/pasteboard/paste-text-011-expected.txt:
        * platform/mac/editing/pasteboard/paste-text-012-expected.txt:
2010-02-17  Tony Chang  <tony@chromium.org>

        Reviewed by Eric Seidel.

        Copying and pasting into a contenteditable area can create <div>s surrounded by <span>s
        https://bugs.webkit.org/show_bug.cgi?id=26937

        This happens because of a span added when we copy that is used to
        preserve styles.  To avoid this, when we paste, make sure to apply
        the styles to the span's children and then remove the style span.

        This change is covered by existing layout tests.

        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::handleStyleSpans):
        (WebCore::ReplaceSelectionCommand::copyStyleToChildren):
        * editing/ReplaceSelectionCommand.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@54933 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 28d03110
2010-02-17 Tony Chang <tony@chromium.org>
Reviewed by Eric Seidel.
https://bugs.webkit.org/show_bug.cgi?id=26937
No longer allow span styles to surround block elements like
divs when pasting (see paste-text-012 and 5065605 which had divs in
spans). This also causes a few cases of empty spans to be removed
(see 19089 and 5245519).
* editing/execCommand/19089-expected.txt:
* editing/pasteboard/5245519-expected.txt:
* editing/pasteboard/5245519.html:
* platform/mac/editing/pasteboard/5065605-expected.txt:
* platform/mac/editing/pasteboard/paste-text-011-expected.txt:
* platform/mac/editing/pasteboard/paste-text-012-expected.txt:
2010-02-17 Tony Chang <tony@chromium.org>
Reviewed by Eric Seidel.
......
This tests to make sure an ASSERT doesn't fire when performing a FormatBlock operation on a selection that ends just after a horizontal rule that is the last element in the document. The test should not assert.
<pre><span class="Apple-style-span" style="font-family: Times; white-space: normal; "><hr></span></pre>
<pre><hr></pre>
This tests for a crash when pasting content that contains Apple-style-spans that don't have renderers.' You should see 'Hello World!' We don't currently remove the empty invisible style span at paste time because it doesn't anticipate encountering non-inheritable styles on style spans, because we never create those at copy time.
Hello <span style="display: none; " class="Apple-style-span"></span>World!
This tests for a crash when pasting content that contains Apple-style-spans that don't have renderers.' You should see 'Hello World!'
Hello World!
<div id="description">This tests for a crash when pasting content that contains Apple-style-spans that don't have renderers.' You should see 'Hello World!' We don't currently remove the empty invisible style span at paste time because it doesn't anticipate encountering non-inheritable styles on style spans, because we never create those at copy time.</div>
<div id="description">This tests for a crash when pasting content that contains Apple-style-spans that don't have renderers.' You should see 'Hello World!'</div>
<div id="edit" contenteditable="true"><br></div>
<script>
......
......@@ -22,12 +22,12 @@ layer at (0,0) size 800x600
RenderInline {FONT} at (0,0) size 148x18 [color=#FF0000]
RenderText {#text} at (0,0) size 148x18
text run at (0,0) width 148: "This text should be red."
RenderBlock (anonymous) at (0,18) size 784x18
RenderBlock {DIV} at (0,0) size 784x18
RenderBlock (anonymous) at (0,18) size 784x18 [color=#FF0000]
RenderBlock {DIV} at (0,0) size 784x18 [color=#000000]
RenderInline {FONT} at (0,0) size 148x18 [color=#FF0000]
RenderText {#text} at (0,0) size 148x18
text run at (0,0) width 148: "This text should be red."
RenderBlock (anonymous) at (0,36) size 784x0
RenderInline {FONT} at (0,0) size 0x0 [color=#FF0000]
RenderInline {SPAN} at (0,0) size 0x0 [color=#000000]
caret: position 24 of child 0 {#text} of child 0 {FONT} of child 1 {DIV} of child 0 {SPAN} of child 0 {FONT} of child 2 {DIV} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
RenderInline {FONT} at (0,0) size 0x0 [color=#FF0000]
caret: position 24 of child 0 {#text} of child 0 {FONT} of child 1 {DIV} of child 0 {FONT} of child 2 {DIV} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
......@@ -9,7 +9,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
layer at (0,0) size 800x600
......@@ -31,7 +31,6 @@ layer at (0,0) size 800x600
RenderBlock (anonymous) at (0,0) size 784x0
RenderInline {FONT} at (0,0) size 0x0
RenderInline {B} at (0,0) size 0x0
RenderInline {SPAN} at (0,0) size 0x0
RenderBlock (anonymous) at (0,0) size 784x58
RenderBlock {P} at (0,0) size 784x21
RenderInline {FONT} at (0,0) size 55x20
......@@ -46,5 +45,6 @@ layer at (0,0) size 800x600
RenderBlock (anonymous) at (0,74) size 784x0
RenderInline {FONT} at (0,0) size 0x0
RenderInline {B} at (0,0) size 0x0
RenderInline {SPAN} at (0,0) size 0x0
caret: position 5 of child 0 {#text} of child 0 {B} of child 0 {FONT} of child 1 {P} of child 0 {SPAN} of child 0 {B} of child 0 {FONT} of child 4 {P} of child 1 {BODY} of child 0 {HTML} of document
RenderInline {FONT} at (0,0) size 0x0
RenderInline {B} at (0,0) size 0x0
caret: position 5 of child 0 {#text} of child 0 {B} of child 0 {FONT} of child 1 {P} of child 0 {B} of child 0 {FONT} of child 4 {P} of child 1 {BODY} of child 0 {HTML} of document
......@@ -7,7 +7,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > SPAN > DIV > BODY > HTML > #document to 0 of DIV > DIV > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
layer at (0,0) size 800x600
......@@ -32,16 +32,12 @@ layer at (0,0) size 800x600
RenderText {#text} at (0,0) size 32x28
text run at (0,0) width 32: "foo"
RenderBlock {DIV} at (0,140) size 784x160 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 756x0
RenderInline {SPAN} at (0,0) size 0x0
RenderBlock (anonymous) at (14,14) size 756x132
RenderBlock {DIV} at (0,0) size 756x132 [border: (2px solid #FF0000)]
RenderBlock {DIV} at (14,38) size 728x28
RenderBlock {BLOCKQUOTE} at (40,0) size 648x28
RenderText {#text} at (0,0) size 32x28
text run at (0,0) width 32: "foo"
RenderBlock {DIV} at (14,90) size 728x28
RenderBR {BR} at (0,0) size 0x28
RenderBlock {DIV} at (14,14) size 756x132 [border: (2px solid #FF0000)]
RenderBlock {DIV} at (14,38) size 728x28
RenderBlock {BLOCKQUOTE} at (40,0) size 648x28
RenderText {#text} at (0,0) size 32x28
text run at (0,0) width 32: "foo"
RenderBlock {DIV} at (14,90) size 728x28
RenderBR {BR} at (0,0) size 0x28
RenderBlock (anonymous) at (14,146) size 756x0
RenderInline {SPAN} at (0,0) size 0x0
caret: position 0 of child 0 {BR} of child 1 {DIV} of child 0 {DIV} of child 0 {SPAN} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
caret: position 0 of child 0 {BR} of child 1 {DIV} of child 0 {DIV} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
2010-02-17 Tony Chang <tony@chromium.org>
Reviewed by Eric Seidel.
Copying and pasting into a contenteditable area can create <div>s surrounded by <span>s
https://bugs.webkit.org/show_bug.cgi?id=26937
This happens because of a span added when we copy that is used to
preserve styles. To avoid this, when we paste, make sure to apply
the styles to the span's children and then remove the style span.
This change is covered by existing layout tests.
* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplaceSelectionCommand::handleStyleSpans):
(WebCore::ReplaceSelectionCommand::copyStyleToChildren):
* editing/ReplaceSelectionCommand.h:
2010-02-17 Tony Chang <tony@chromium.org>
Reviewed by Eric Seidel.
......
......@@ -649,10 +649,11 @@ void ReplaceSelectionCommand::handleStyleSpans()
}
// There are non-redundant styles on sourceDocumentStyleSpan, but there is no
// copiedRangeStyleSpan. Clear the redundant styles from sourceDocumentStyleSpan
// and return.
// copiedRangeStyleSpan. Remove the span, because it could be surrounding block elements,
// and apply the styles to its children.
if (sourceDocumentStyle->length() > 0 && !copiedRangeStyleSpan) {
setNodeAttribute(static_cast<Element*>(sourceDocumentStyleSpan), styleAttr, sourceDocumentStyle->cssText());
copyStyleToChildren(sourceDocumentStyleSpan, sourceDocumentStyle.get());
removeNodePreservingChildren(sourceDocumentStyleSpan);
return;
}
......@@ -684,6 +685,34 @@ void ReplaceSelectionCommand::handleStyleSpans()
setNodeAttribute(static_cast<Element*>(copiedRangeStyleSpan), styleAttr, copiedRangeStyle->cssText());
}
// Take the style attribute of a span and apply it to it's children instead. This allows us to
// convert invalid HTML where a span contains block elements into valid HTML while preserving
// styles.
void ReplaceSelectionCommand::copyStyleToChildren(Node* parentNode, const CSSMutableStyleDeclaration* parentStyle)
{
ASSERT(parentNode->hasTagName(spanTag));
for (Node* childNode = parentNode->firstChild(); childNode; childNode = childNode->nextSibling()) {
if (childNode->isTextNode() || !isBlock(childNode) || childNode->hasTagName(preTag)) {
// In this case, put a span tag around the child node.
RefPtr<Node> newSpan = parentNode->cloneNode(false);
setNodeAttribute(static_cast<Element*>(newSpan.get()), styleAttr, parentStyle->cssText());
insertNodeAfter(newSpan, childNode);
ExceptionCode ec = 0;
newSpan->appendChild(childNode, ec);
ASSERT(!ec);
childNode = newSpan.get();
} else if (childNode->isHTMLElement()) {
// Copy the style attribute and merge them into the child node. We don't want to override
// existing styles, so don't clobber on merge.
RefPtr<CSSMutableStyleDeclaration> newStyle = parentStyle->copy();
HTMLElement* childElement = static_cast<HTMLElement*>(childNode);
RefPtr<CSSMutableStyleDeclaration> existingStyles = childElement->getInlineStyleDecl()->copy();
existingStyles->merge(newStyle.get(), false);
setNodeAttribute(childElement, styleAttr, existingStyles->cssText());
}
}
}
void ReplaceSelectionCommand::mergeEndIfNeeded()
{
if (!m_shouldMergeEnd)
......
......@@ -69,6 +69,7 @@ private:
void negateStyleRulesThatAffectAppearance();
void handleStyleSpans();
void copyStyleToChildren(Node* parentNode, const CSSMutableStyleDeclaration* parentStyle);
void handlePasteAsQuotationNode();
virtual void removeNodePreservingChildren(Node*);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment