Diff
Modified: trunk/LayoutTests/ChangeLog (98793 => 98794)
--- trunk/LayoutTests/ChangeLog 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/LayoutTests/ChangeLog 2011-10-29 05:27:07 UTC (rev 98794)
@@ -1,3 +1,26 @@
+2011-10-28 Ryosuke Niwa <[email protected]>
+
+ The copy and paste result in nested scrollbars on http://dojotoolkit.org/widgets
+ https://bugs.webkit.org/show_bug.cgi?id=70799
+
+ Reviewed by Enrica Casucci.
+
+ Added a test to copy contents inside a body with background and text-decoration properties.
+
+ WebKit should not copy body element.
+
+ * editing/deleting/delete-line-break-before-underlined-content-expected.txt: an erroneous inline div
+ is replaced by a span.
+ * editing/deleting/deleting-line-break-preserves-underline-color-expected.txt: two style spans are
+ merged into one.
+ * editing/pasteboard/19644-2-expected.txt: div is replaced by span. This is okay because it's the only
+ content in the body. Even though we now only put the gray background under text as inline style as
+ opposed to apply at the block level, that's what execCommand('BackColor'...) does and what user expects.
+ * editing/pasteboard/avoid-copying-body-with-background-expected.txt: Added.
+ * editing/pasteboard/avoid-copying-body-with-background.html: Added.
+ * editing/pasteboard/preserve-underline-color-expected.txt:
+ * platform/mac/editing/pasteboard/5134759-expected.txt:
+
2011-10-28 Adam Klein <[email protected]>
[MutationObservers] Support attributeOldValue for attribute mutations
Modified: trunk/LayoutTests/editing/deleting/delete-line-break-before-underlined-content-expected.txt (98793 => 98794)
--- trunk/LayoutTests/editing/deleting/delete-line-break-before-underlined-content-expected.txt 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/LayoutTests/editing/deleting/delete-line-break-before-underlined-content-expected.txt 2011-10-29 05:27:07 UTC (rev 98794)
@@ -1,6 +1,5 @@
This tests for a bug where underlined content would lose its underliningwhen deleting the line break before the paragraph that contained it.
| "This shouldn't be underlined.<#selection-caret>"
-| <div>
-| id="div"
-| style="text-decoration: underline; display: inline !important; "
+| <span>
+| style="text-decoration: underline; "
| "This should be underlined."
Modified: trunk/LayoutTests/editing/deleting/deleting-line-break-preserves-underline-color-expected.txt (98793 => 98794)
--- trunk/LayoutTests/editing/deleting/deleting-line-break-preserves-underline-color-expected.txt 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/LayoutTests/editing/deleting/deleting-line-break-preserves-underline-color-expected.txt 2011-10-29 05:27:07 UTC (rev 98794)
@@ -13,7 +13,5 @@
| <div>
| "This should not be underlined.<#selection-caret>"
| <span>
-| style="text-decoration: underline; color: blue;"
-| <span>
-| style="color:red;"
-| "This should be underlined."
+| style="color: red; text-decoration: underline; "
+| "This should be underlined."
Modified: trunk/LayoutTests/editing/pasteboard/19644-2-expected.txt (98793 => 98794)
--- trunk/LayoutTests/editing/pasteboard/19644-2-expected.txt 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/LayoutTests/editing/pasteboard/19644-2-expected.txt 2011-10-29 05:27:07 UTC (rev 98794)
@@ -1 +1 @@
-<div style="background-color: rgb(187, 187, 187); ">This tests to make sure that we wrap copied markup in a div to hold a fully selected body's attributes and style when the that body has a background-color. If you copy and paste this text into Mail, it should have a grey background.</div>
+<span style="background-color: rgb(187, 187, 187); ">This tests to make sure that we wrap copied markup in a div to hold a fully selected body's attributes and style when the that body has a background-color. If you copy and paste this text into Mail, it should have a grey background.</span>
Added: trunk/LayoutTests/editing/pasteboard/avoid-copying-body-with-background-expected.txt (0 => 98794)
--- trunk/LayoutTests/editing/pasteboard/avoid-copying-body-with-background-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/avoid-copying-body-with-background-expected.txt 2011-10-29 05:27:07 UTC (rev 98794)
@@ -0,0 +1,29 @@
+This test copies and pastes content inside a body with background and text-decoration properties. WebKit should not nest body.
+| "
+"
+| <ul>
+| "
+"
+| <li>
+| "hello, world "
+| <a>
+| href=""
+| "WebKit"
+| "
+"
+| <li>
+| "Copied content"
+| <li>
+| "hello, world"
+| " "
+| <a>
+| href=""
+| "WebKit"
+| <li>
+| "Copied content<#selection-caret>"
+| "
+"
+| "
+"
+| "
+"
Added: trunk/LayoutTests/editing/pasteboard/avoid-copying-body-with-background.html (0 => 98794)
--- trunk/LayoutTests/editing/pasteboard/avoid-copying-body-with-background.html (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/avoid-copying-body-with-background.html 2011-10-29 05:27:07 UTC (rev 98794)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ background: transparent;
+ width: 500px;
+ height: 200px;
+ overflow-x: auto; overflow-y:auto;
+ text-decoration: underline;
+}
+</style>
+</head>
+<body style="color: rgb(51, 51, 51); " contenteditable="true">
+<ul>
+<li>hello, world <a href=""
+<li>Copied content</li>
+</ul>
+<script src=""
+<script>
+
+document.body.focus();
+document.execCommand('SelectAll', false, null);
+document.execCommand('Copy', false, null);
+getSelection().modify('Move', 'Forward', 'Character');
+document.execCommand('Paste', false, null);
+
+while (script = document.querySelector('script'))
+ script.parentNode.removeChild(script);
+
+Markup.description('This test copies and pastes content inside a body with background and text-decoration properties. WebKit should not nest body.');
+Markup.dump(document.body);
+
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/editing/pasteboard/preserve-underline-color-expected.txt (98793 => 98794)
--- trunk/LayoutTests/editing/pasteboard/preserve-underline-color-expected.txt 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/LayoutTests/editing/pasteboard/preserve-underline-color-expected.txt 2011-10-29 05:27:07 UTC (rev 98794)
@@ -1,7 +1,5 @@
This test for a bug copy/pasting underlined text. The color of the underline should be the color of the element that has the text-decoration property.
| <span>
-| style="text-decoration: underline; color: blue; "
-| <span>
-| style="color: red; "
-| "This should be underlined.<#selection-caret>"
+| style="color: rgb(255, 0, 0); text-decoration: underline; "
+| "This should be underlined.<#selection-caret>"
| <br>
Modified: trunk/LayoutTests/platform/mac/editing/pasteboard/5134759-expected.txt (98793 => 98794)
--- trunk/LayoutTests/platform/mac/editing/pasteboard/5134759-expected.txt 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/LayoutTests/platform/mac/editing/pasteboard/5134759-expected.txt 2011-10-29 05:27:07 UTC (rev 98794)
@@ -17,8 +17,7 @@
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!"
+ 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 body
+caret: position 6 of child 0 {#text} of child 1 {SPAN} of child 0 {DIV} of child 2 {DIV} of body
Modified: trunk/Source/WebCore/ChangeLog (98793 => 98794)
--- trunk/Source/WebCore/ChangeLog 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/Source/WebCore/ChangeLog 2011-10-29 05:27:07 UTC (rev 98794)
@@ -1,3 +1,38 @@
+2011-10-28 Ryosuke Niwa <[email protected]>
+
+ The copy and paste result in nested scrollbars on http://dojotoolkit.org/widgets
+ https://bugs.webkit.org/show_bug.cgi?id=70799
+
+ Reviewed by Enrica Casucci.
+
+ The bug was caused by WebKit's treating a fully selected root with background property as a special common ancestor.
+ A variant of this bug was caused by treating any element with text-decoration property as a presentational element.
+
+ Fixed the above two bugs by not serializing the said nodes. The effective background color was already serialized
+ by wrappingStyleForSerialization, there was nothing to be done besides stop including it in highestAncestorToWrapMarkup.
+
+ For text-decoration property, added the logic to compute the effective value in EditingStyle::init. Also treat it
+ as a non-inheritable editing property so that the rest of EditingStyle just works.
+
+ Test: editing/pasteboard/avoid-copying-body-with-background.html
+
+ * editing/EditingStyle.cpp: Added CSSPropertyTextDecoration to the list of editing properties.
+ (WebCore::copyEditingProperties):
+ (WebCore::EditingStyle::init): Compute the effective text decoration when propertiesToInclude is
+ EditingPropertiesInEffect.
+ (WebCore::EditingStyle::prepareToApplyAt):
+ (WebCore::EditingStyle::mergeInlineStyleOfElement):
+ (WebCore::EditingStyle::wrappingStyleForSerialization):
+ (WebCore::EditingStyle::removeStyleFromRulesAndContext):
+ * editing/EditingStyle.h: Renamed EditingInheritablePropertiesAndBackgroundColorInEffect to
+ EditingPropertiesInEffect.
+ * editing/markup.cpp:
+ (WebCore::StyledMarkupAccumulator::appendStyleNodeOpenTag): Removed an assertion that's no longer valid.
+ (WebCore::isElementPresentational): Don't consider an element with text-decoration as a presentational element.
+ (WebCore::highestAncestorToWrapMarkup): Don't consider fully selected root as a special common ancestor ever.
+ Background color is computed property when we compute the wrapping style.
+ (WebCore::createMarkup):
+
2011-10-28 Adam Barth <[email protected]>
Rename ExceptionCodeDescription.in to DOMExceptions.in
Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (98793 => 98794)
--- trunk/Source/WebCore/editing/EditingStyle.cpp 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp 2011-10-29 05:27:07 UTC (rev 98794)
@@ -55,6 +55,7 @@
// e.g. when a user inserts a new paragraph, all properties listed here must be copied to the new paragraph.
static const int editingProperties[] = {
CSSPropertyBackgroundColor,
+ CSSPropertyTextDecoration,
// CSS inheritable properties
CSSPropertyColor,
@@ -79,11 +80,11 @@
CSSPropertyWebkitTextStrokeWidth,
};
-static PassRefPtr<CSSMutableStyleDeclaration> copyEditingProperties(CSSStyleDeclaration* style, bool includeBackgroundColor = false)
+static PassRefPtr<CSSMutableStyleDeclaration> copyEditingProperties(CSSStyleDeclaration* style, bool includeNonInheritableProperties = false)
{
- if (includeBackgroundColor)
+ if (includeNonInheritableProperties)
return style->copyPropertiesInSet(editingProperties, WTF_ARRAY_LENGTH(editingProperties));
- return style->copyPropertiesInSet(editingProperties + 1, WTF_ARRAY_LENGTH(editingProperties) - 1);
+ return style->copyPropertiesInSet(editingProperties + 2, WTF_ARRAY_LENGTH(editingProperties) - 2);
}
static inline bool isEditingProperty(int id)
@@ -362,9 +363,11 @@
RefPtr<CSSComputedStyleDeclaration> computedStyleAtPosition = computedStyle(node);
m_mutableStyle = propertiesToInclude == AllProperties && computedStyleAtPosition ? computedStyleAtPosition->copy() : editingStyleFromComputedStyle(computedStyleAtPosition);
- if (propertiesToInclude == EditingInheritablePropertiesAndBackgroundColorInEffect) {
+ if (propertiesToInclude == EditingPropertiesInEffect) {
if (RefPtr<CSSValue> value = backgroundColorInEffect(node))
m_mutableStyle->setProperty(CSSPropertyBackgroundColor, value->cssText());
+ if (RefPtr<CSSValue> value = computedStyleAtPosition->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect))
+ m_mutableStyle->setProperty(CSSPropertyTextDecoration, value->cssText());
}
if (node && node->computedStyle()) {
@@ -854,7 +857,7 @@
// ReplaceSelectionCommand::handleStyleSpans() requires that this function only removes the editing style.
// If this function was modified in the future to delete all redundant properties, then add a boolean value to indicate
// which one of editingStyleAtPosition or computedStyle is called.
- RefPtr<EditingStyle> style = EditingStyle::create(position, EditingInheritablePropertiesAndBackgroundColorInEffect);
+ RefPtr<EditingStyle> style = EditingStyle::create(position, EditingPropertiesInEffect);
RefPtr<CSSValue> unicodeBidi;
RefPtr<CSSValue> direction;
@@ -900,8 +903,8 @@
mergeStyle(element->inlineStyleDecl(), mode);
return;
case OnlyEditingInheritableProperties:
- case EditingInheritablePropertiesAndBackgroundColorInEffect:
- mergeStyle(copyEditingProperties(element->inlineStyleDecl(), propertiesToInclude == EditingInheritablePropertiesAndBackgroundColorInEffect).get(), mode);
+ case EditingPropertiesInEffect:
+ mergeStyle(copyEditingProperties(element->inlineStyleDecl(), propertiesToInclude == EditingPropertiesInEffect).get(), mode);
return;
}
}
@@ -936,7 +939,7 @@
{
RefPtr<EditingStyle> wrappingStyle;
if (shouldAnnotate) {
- wrappingStyle = EditingStyle::create(context, EditingStyle::EditingInheritablePropertiesAndBackgroundColorInEffect);
+ wrappingStyle = EditingStyle::create(context, EditingStyle::EditingPropertiesInEffect);
// Styles that Mail blockquotes contribute should only be placed on the Mail blockquote,
// to help us differentiate those styles from ones that the user has applied.
@@ -955,7 +958,7 @@
for (Node* node = context; node && !node->isDocumentNode(); node = node->parentNode()) {
if (node->isStyledElement()) {
wrappingStyle->mergeInlineAndImplicitStyleOfElement(static_cast<StyledElement*>(node), EditingStyle::DoNotOverrideValues,
- EditingStyle::EditingInheritablePropertiesAndBackgroundColorInEffect);
+ EditingStyle::EditingPropertiesInEffect);
}
}
@@ -1068,7 +1071,7 @@
m_mutableStyle = getPropertiesNotIn(m_mutableStyle.get(), styleFromMatchedRules.get());
// 2. Remove style present in context and not overriden by matched rules.
- RefPtr<EditingStyle> computedStyle = EditingStyle::create(context, EditingInheritablePropertiesAndBackgroundColorInEffect);
+ RefPtr<EditingStyle> computedStyle = EditingStyle::create(context, EditingPropertiesInEffect);
if (computedStyle->m_mutableStyle) {
computedStyle->removePropertiesInElementDefaultStyle(element);
m_mutableStyle = getPropertiesNotIn(m_mutableStyle.get(), computedStyle->m_mutableStyle.get());
Modified: trunk/Source/WebCore/editing/EditingStyle.h (98793 => 98794)
--- trunk/Source/WebCore/editing/EditingStyle.h 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/Source/WebCore/editing/EditingStyle.h 2011-10-29 05:27:07 UTC (rev 98794)
@@ -61,7 +61,7 @@
class EditingStyle : public RefCounted<EditingStyle> {
public:
- enum PropertiesToInclude { AllProperties, OnlyEditingInheritableProperties, EditingInheritablePropertiesAndBackgroundColorInEffect };
+ enum PropertiesToInclude { AllProperties, OnlyEditingInheritableProperties, EditingPropertiesInEffect };
enum ShouldPreserveWritingDirection { PreserveWritingDirection, DoNotPreserveWritingDirection };
enum ShouldExtractMatchingStyle { ExtractMatchingStyle, DoNotExtractMatchingStyle };
static float NoFontDelta;
Modified: trunk/Source/WebCore/editing/markup.cpp (98793 => 98794)
--- trunk/Source/WebCore/editing/markup.cpp 2011-10-29 01:56:13 UTC (rev 98793)
+++ trunk/Source/WebCore/editing/markup.cpp 2011-10-29 05:27:07 UTC (rev 98794)
@@ -185,9 +185,8 @@
void StyledMarkupAccumulator::appendStyleNodeOpenTag(StringBuilder& out, CSSStyleDeclaration* style, Document* document, bool isBlock)
{
- // All text-decoration-related elements should have been treated as special ancestors
- // If we ever hit this ASSERT, we should export StyleChange in ApplyStyleCommand and use it here
- ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyTextDecoration) && propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect));
+ // wrappingStyleForSerialization should have removed -webkit-text-decorations-in-effect
+ ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect));
DEFINE_STATIC_LOCAL(const String, divStyle, ("<div style=\""));
DEFINE_STATIC_LOCAL(const String, styleSpanOpen, ("<span style=\""));
out.append(isBlock ? divStyle : styleSpanOpen);
@@ -519,27 +518,12 @@
static bool isElementPresentational(const Node* node)
{
- if (node->hasTagName(uTag) || node->hasTagName(sTag) || node->hasTagName(strikeTag)
- || node->hasTagName(iTag) || node->hasTagName(emTag) || node->hasTagName(bTag) || node->hasTagName(strongTag))
- return true;
- RefPtr<EditingStyle> style = styleFromMatchedRulesAndInlineDecl(node);
- return style && style->style() && !propertyMissingOrEqualToNone(style->style(), CSSPropertyTextDecoration);
+ return node->hasTagName(uTag) || node->hasTagName(sTag) || node->hasTagName(strikeTag)
+ || node->hasTagName(iTag) || node->hasTagName(emTag) || node->hasTagName(bTag) || node->hasTagName(strongTag);
}
-static bool shouldIncludeWrapperForFullySelectedRoot(Node* fullySelectedRoot)
+static Node* highestAncestorToWrapMarkup(const Range* range, EAnnotateForInterchange shouldAnnotate)
{
- if (fullySelectedRoot->isElementNode() && static_cast<Element*>(fullySelectedRoot)->hasAttribute(backgroundAttr))
- return true;
-
- RefPtr<EditingStyle> style = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
- if (!style || !style->style())
- return false;
-
- return style->style()->getPropertyCSSValue(CSSPropertyBackgroundImage) || style->style()->getPropertyCSSValue(CSSPropertyBackgroundColor);
-}
-
-static Node* highestAncestorToWrapMarkup(const Range* range, Node* fullySelectedRoot, EAnnotateForInterchange shouldAnnotate)
-{
ExceptionCode ec;
Node* commonAncestor = range->commonAncestorContainer(ec);
ASSERT(commonAncestor);
@@ -573,9 +557,6 @@
if (Node *enclosingAnchor = enclosingNodeWithTag(firstPositionInNode(specialCommonAncestor ? specialCommonAncestor : commonAncestor), aTag))
specialCommonAncestor = enclosingAnchor;
- if (shouldAnnotate == AnnotateForInterchange && fullySelectedRoot && shouldIncludeWrapperForFullySelectedRoot(fullySelectedRoot))
- specialCommonAncestor = fullySelectedRoot;
-
return specialCommonAncestor;
}
@@ -620,7 +601,7 @@
// FIXME: Do this for all fully selected blocks, not just the body.
if (body && areRangesEqual(VisibleSelection::selectionFromContentsOfNode(body).toNormalizedRange().get(), range))
fullySelectedRoot = body;
- Node* specialCommonAncestor = highestAncestorToWrapMarkup(updatedRange.get(), fullySelectedRoot, shouldAnnotate);
+ Node* specialCommonAncestor = highestAncestorToWrapMarkup(updatedRange.get(), shouldAnnotate);
StyledMarkupAccumulator accumulator(nodes, shouldResolveURLs, shouldAnnotate, updatedRange.get(), specialCommonAncestor);
Node* pastEnd = updatedRange->pastLastNode();
