Diff
Modified: trunk/LayoutTests/ChangeLog (92329 => 92330)
--- trunk/LayoutTests/ChangeLog 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/LayoutTests/ChangeLog 2011-08-03 23:42:25 UTC (rev 92330)
@@ -1,3 +1,18 @@
+2011-08-03 Ryosuke Niwa <rn...@webkit.org>
+
+ select-all, copy, paste of specialAncestorElements (e.g. pre, h1, etc) nests the element inside itself
+ https://bugs.webkit.org/show_bug.cgi?id=26483
+
+ Reviewed by Enrica Casucci.
+
+ * editing/pasteboard/5065605-expected.txt:
+ * editing/pasteboard/copy-paste-text-in-h1-expected.txt: Added.
+ * editing/pasteboard/copy-paste-text-in-h1.html: Added.
+ * editing/pasteboard/display-block-on-spans-expected.txt:
+ * editing/pasteboard/paste-pre-001-expected.txt:
+ * editing/pasteboard/paste-pre-002-expected.txt:
+ * editing/pasteboard/paste-text-011-expected.txt:
+
2011-08-03 Mark Pilgrim <pilg...@chromium.org>
Remove LegacyDefaultOptionalArguments flag from HTML DOM IDL files
Modified: trunk/LayoutTests/editing/pasteboard/5065605-expected.txt (92329 => 92330)
--- trunk/LayoutTests/editing/pasteboard/5065605-expected.txt 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/LayoutTests/editing/pasteboard/5065605-expected.txt 2011-08-03 23:42:25 UTC (rev 92330)
@@ -21,7 +21,7 @@
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -36,19 +36,12 @@
| class="Apple-style-span"
| color="#ff0000"
| "This text should be red."
+| <font>
+| class="Apple-style-span"
+| color="#ff0000"
+| "This text should be red."
| <div>
| <font>
| class="Apple-style-span"
| color="#ff0000"
-| <span>
-| class="Apple-style-span"
-| style="color: rgb(0, 0, 0); "
-| <font>
-| class="Apple-style-span"
-| color="#ff0000"
-| "This text should be red."
-| <div>
-| <font>
-| class="Apple-style-span"
-| color="#ff0000"
-| "This text should be red.<#selection-caret>"
+| "This text should be red.<#selection-caret>"
Added: trunk/LayoutTests/editing/pasteboard/copy-paste-text-in-h1-expected.txt (0 => 92330)
--- trunk/LayoutTests/editing/pasteboard/copy-paste-text-in-h1-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/copy-paste-text-in-h1-expected.txt 2011-08-03 23:42:25 UTC (rev 92330)
@@ -0,0 +1,11 @@
+This tests copying and pasting text in h1. WebKit should not nest h1s.
+To manually test, copy and paste "hello world". You should see single border around "hello world".
+| "
+"
+| <h1>
+| "hello"
+| " "
+| <em>
+| "world<#selection-caret>"
+| "
+"
Added: trunk/LayoutTests/editing/pasteboard/copy-paste-text-in-h1.html (0 => 92330)
--- trunk/LayoutTests/editing/pasteboard/copy-paste-text-in-h1.html (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/copy-paste-text-in-h1.html 2011-08-03 23:42:25 UTC (rev 92330)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+h1 { border: solid 1px black; padding: 5px; }
+</style>
+</head>
+<body>
+<p id="description">This tests copying and pasting text in h1. WebKit should not nest h1s.
+To manually test, copy and paste "hello world". You should see single border around "hello world".</p>
+<div id="test" contenteditable>
+<h1>hello <em>world</em></h1>
+</div>
+<script src=""
+<script>
+
+var test = document.getElementById('test');
+test.focus();
+
+Markup.description(document.getElementById('description').textContent);
+
+document.execCommand('SelectAll', false, null);
+document.execCommand('Copy', false, null);
+document.execCommand('Paste', false, null);
+
+if (window.layoutTestController)
+ Markup.dump(test);
+else
+ Markup.noAutoDump();
+
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/editing/pasteboard/display-block-on-spans-expected.txt (92329 => 92330)
--- trunk/LayoutTests/editing/pasteboard/display-block-on-spans-expected.txt 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/LayoutTests/editing/pasteboard/display-block-on-spans-expected.txt 2011-08-03 23:42:25 UTC (rev 92330)
@@ -29,8 +29,11 @@
| <span>
| style="display:block"
| <b>
-| "This<#selection-caret>"
-| <b>
+| <span>
+| class="Apple-style-span"
+| style="font-weight: normal; "
+| <b>
+| "This<#selection-caret>"
| " is another paragraph."
| <br>
| "
Modified: trunk/LayoutTests/editing/pasteboard/paste-pre-001-expected.txt (92329 => 92330)
--- trunk/LayoutTests/editing/pasteboard/paste-pre-001-expected.txt 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/LayoutTests/editing/pasteboard/paste-pre-001-expected.txt 2011-08-03 23:42:25 UTC (rev 92330)
@@ -7,4 +7,4 @@
foo
bar
execCutCommand: <div id="test" class="editing"> <pre><br></pre> </div>
-execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: Times; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
+execPasteCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
Modified: trunk/LayoutTests/editing/pasteboard/paste-pre-002-expected.txt (92329 => 92330)
--- trunk/LayoutTests/editing/pasteboard/paste-pre-002-expected.txt 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/LayoutTests/editing/pasteboard/paste-pre-002-expected.txt 2011-08-03 23:42:25 UTC (rev 92330)
@@ -2,4 +2,4 @@
foo
bar
execCopyCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
-execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: Times; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
+execPasteCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
Modified: trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt (92329 => 92330)
--- trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt 2011-08-03 23:42:25 UTC (rev 92330)
@@ -6,7 +6,7 @@
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 > BODY > HTML > #document to 5 of #text > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -30,19 +30,12 @@
| <font>
| face="Monaco"
| <b>
-| <span>
-| class="Apple-style-span"
-| style="font-family: Times; font-weight: normal; "
-| <p>
-| <font>
-| face="Monaco"
-| <b>
-| "hello"
-| <p>
-| <font>
-| face="Monaco"
-| <b>
-| "there<#selection-caret>"
+| "hello"
+| <p>
+| <font>
+| face="Monaco"
+| <b>
+| "there<#selection-caret>"
| "
Modified: trunk/Source/WebCore/ChangeLog (92329 => 92330)
--- trunk/Source/WebCore/ChangeLog 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/Source/WebCore/ChangeLog 2011-08-03 23:42:25 UTC (rev 92330)
@@ -1,3 +1,26 @@
+2011-08-03 Ryosuke Niwa <rn...@webkit.org>
+
+ select-all, copy, paste of specialAncestorElements (e.g. pre, h1, etc) nests the element inside itself
+ https://bugs.webkit.org/show_bug.cgi?id=26483
+
+ Reviewed by Enrica Casucci.
+
+ The bug was caused by WebKit serializing pre, h1, etc... to retain structure and appearance when copying
+ rich content and pasting did not remove such nodes wrapping the copied contents.
+
+ Fixed the bug by extending r81887 and r83322 to remove those elements from where WebKit pastes into.
+
+ Test: editing/pasteboard/copy-paste-text-in-h1.html
+
+ * editing/ReplaceSelectionCommand.cpp:
+ (WebCore::nodeHasAttributesToPreserve): Extracted from isInlineNodeWithStyle.
+ (WebCore::isInlineNodeWithStyle): Calls nodeHasAttributesToPreserve.
+ (WebCore::ReplaceSelectionCommand::doApply): Calls ancestorToRetainStructureAndAppearance.
+ Remove nodes copied by ancestorToRetainStructureAndAppearance at insertionPos before pasting the fragment.
+ * editing/markup.cpp:
+ (WebCore::ancestorToRetainStructureAndAppearance): Takes ShouldIncludeParagraphSeparators.
+ * editing/markup.h:
+
2011-08-03 Mark Pilgrim <pilg...@chromium.org>
Remove LegacyDefaultOptionalArguments flag from Console.idl
Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp (92329 => 92330)
--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2011-08-03 23:42:25 UTC (rev 92330)
@@ -760,6 +760,12 @@
return node;
}
+static bool nodeHasAttributesToPreserve(const HTMLElement* element)
+{
+ const NamedNodeMap* attributeMap = element->attributeMap();
+ return attributeMap && !attributeMap->isEmpty() && (attributeMap->length() > 1 || !element->hasAttribute(styleAttr));
+}
+
static bool isInlineNodeWithStyle(const Node* node)
{
// We don't want to skip over any block elements.
@@ -781,11 +787,7 @@
// We can skip inline elements that don't have attributes or whose only
// attribute is the style attribute.
- const NamedNodeMap* attributeMap = element->attributeMap();
- if (!attributeMap || attributeMap->isEmpty() || (attributeMap->length() == 1 && element->hasAttribute(styleAttr)))
- return true;
-
- return false;
+ return !nodeHasAttributesToPreserve(static_cast<const HTMLElement*>(node));
}
void ReplaceSelectionCommand::doApply()
@@ -937,16 +939,21 @@
// We can skip this optimization for fragments not wrapped in one of
// our style spans and for positions inside list items
// since insertAsListItems already does the right thing.
- if (!m_matchStyle && !enclosingList(insertionPos.containerNode()) && isStyleSpan(fragment.firstChild())) {
+ if (!m_matchStyle && !enclosingList(insertionPos.containerNode()) && isStyleSpan(fragment.firstChild())
+ && VisiblePosition(firstPositionInNode(insertionPos.containerNode())) == VisiblePosition(lastPositionInNode(insertionPos.containerNode()))) {
if (insertionPos.containerNode()->isTextNode() && insertionPos.offsetInContainerNode() && !insertionPos.atLastEditingPositionForNode()) {
splitTextNodeContainingElement(insertionPos.containerText(), insertionPos.offsetInContainerNode());
insertionPos = firstPositionInNode(insertionPos.containerNode());
}
- // FIXME: isInlineNodeWithStyle does not check editability.
- if (RefPtr<Node> nodeToSplitTo = highestEnclosingNodeOfType(insertionPos, isInlineNodeWithStyle)) {
- if (insertionPos.containerNode() != nodeToSplitTo) {
- nodeToSplitTo = splitTreeToNode(insertionPos.anchorNode(), nodeToSplitTo.get(), true).get();
+ RefPtr<Node> nodeToSplitTo = highestEnclosingNodeOfType(insertionPos, isInlineNodeWithStyle);
+ if (HTMLElement* ancestor = ancestorToRetainStructureAndAppearance(nodeToSplitTo ? nodeToSplitTo.get() : insertionPos.containerNode(), IncludeParagraphSeparators)) {
+ if (ancestor->parentNode() && unsplittableElementForPosition(insertionPos)->contains(ancestor->parentNode()) && !nodeHasAttributesToPreserve(ancestor))
+ nodeToSplitTo = ancestor;
+ }
+ if (nodeToSplitTo) {
+ if (insertionPos.containerNode() != nodeToSplitTo->parentNode()) {
+ nodeToSplitTo = splitTreeToNode(insertionPos.anchorNode(), nodeToSplitTo->parentNode()).get();
insertionPos = positionInParentBeforeNode(nodeToSplitTo.get());
}
}
Modified: trunk/Source/WebCore/editing/markup.cpp (92329 => 92330)
--- trunk/Source/WebCore/editing/markup.cpp 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/Source/WebCore/editing/markup.cpp 2011-08-03 23:42:25 UTC (rev 92330)
@@ -350,7 +350,7 @@
return lastClosed;
}
-static Node* ancestorToRetainStructureAndAppearance(Node* commonAncestor)
+HTMLElement* ancestorToRetainStructureAndAppearance(Node* commonAncestor, ShouldIncludeParagraphSeparators shouldIncludeParagraphSeparators)
{
Node* commonAncestorBlock = enclosingBlock(commonAncestor);
@@ -362,7 +362,7 @@
while (table && !table->hasTagName(tableTag))
table = table->parentNode();
- return table;
+ return toHTMLElement(table);
}
if (commonAncestorBlock->hasTagName(listingTag)
@@ -376,8 +376,12 @@
|| commonAncestorBlock->hasTagName(h3Tag)
|| commonAncestorBlock->hasTagName(h4Tag)
|| commonAncestorBlock->hasTagName(h5Tag))
- return commonAncestorBlock;
+ return toHTMLElement(commonAncestorBlock);
+ if (shouldIncludeParagraphSeparators == IncludeParagraphSeparators
+ && (commonAncestorBlock->hasTagName(pTag) || commonAncestorBlock->hasTagName(divTag)))
+ return toHTMLElement(commonAncestorBlock);
+
return 0;
}
Modified: trunk/Source/WebCore/editing/markup.h (92329 => 92330)
--- trunk/Source/WebCore/editing/markup.h 2011-08-03 23:35:03 UTC (rev 92329)
+++ trunk/Source/WebCore/editing/markup.h 2011-08-03 23:42:25 UTC (rev 92330)
@@ -33,31 +33,35 @@
namespace WebCore {
- class Document;
- class DocumentFragment;
- class Element;
- class KURL;
- class Node;
- class Range;
+class Document;
+class DocumentFragment;
+class Element;
+class HTMLElement;
+class KURL;
+class Node;
+class Range;
- enum EChildrenOnly { IncludeNode, ChildrenOnly };
- enum EAbsoluteURLs { DoNotResolveURLs, AbsoluteURLs };
+enum EChildrenOnly { IncludeNode, ChildrenOnly };
+enum EAbsoluteURLs { DoNotResolveURLs, AbsoluteURLs };
- PassRefPtr<DocumentFragment> createFragmentFromText(Range* context, const String& text);
- PassRefPtr<DocumentFragment> createFragmentFromMarkup(Document*, const String& markup, const String& baseURL, FragmentScriptingPermission = FragmentScriptingAllowed);
- PassRefPtr<DocumentFragment> createFragmentFromNodes(Document*, const Vector<Node*>&);
+PassRefPtr<DocumentFragment> createFragmentFromText(Range* context, const String& text);
+PassRefPtr<DocumentFragment> createFragmentFromMarkup(Document*, const String& markup, const String& baseURL, FragmentScriptingPermission = FragmentScriptingAllowed);
+PassRefPtr<DocumentFragment> createFragmentFromNodes(Document*, const Vector<Node*>&);
- bool isPlainTextMarkup(Node *node);
+bool isPlainTextMarkup(Node*);
- String createMarkup(const Range*,
- Vector<Node*>* = 0, EAnnotateForInterchange = DoNotAnnotateForInterchange, bool convertBlocksToInlines = false, EAbsoluteURLs = DoNotResolveURLs);
- String createMarkup(const Node*, EChildrenOnly = IncludeNode, Vector<Node*>* = 0, EAbsoluteURLs = DoNotResolveURLs);
-
- String createFullMarkup(const Node*);
- String createFullMarkup(const Range*);
+String createMarkup(const Range*, Vector<Node*>* = 0, EAnnotateForInterchange = DoNotAnnotateForInterchange, bool convertBlocksToInlines = false, EAbsoluteURLs = DoNotResolveURLs);
+String createMarkup(const Node*, EChildrenOnly = IncludeNode, Vector<Node*>* = 0, EAbsoluteURLs = DoNotResolveURLs);
- String urlToMarkup(const KURL&, const String& title);
- String imageToMarkup(const KURL&, Element*);
+String createFullMarkup(const Node*);
+String createFullMarkup(const Range*);
+
+String urlToMarkup(const KURL&, const String& title);
+String imageToMarkup(const KURL&, Element*);
+
+enum ShouldIncludeParagraphSeparators { DoNotIncludeParagraphSeparators, IncludeParagraphSeparators };
+HTMLElement* ancestorToRetainStructureAndAppearance(Node*, ShouldIncludeParagraphSeparators = DoNotIncludeParagraphSeparators);
+
}
#endif // markup_h
