Diff
Modified: trunk/LayoutTests/ChangeLog (143820 => 143821)
--- trunk/LayoutTests/ChangeLog 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/LayoutTests/ChangeLog 2013-02-23 01:54:30 UTC (rev 143821)
@@ -1,3 +1,24 @@
+2013-02-22 Ryosuke Niwa <[email protected]>
+
+ WebKit can erroneously strip font-size CSS property from font element with size attribute
+ https://bugs.webkit.org/show_bug.cgi?id=110657
+
+ Reviewed by Justin Garcia.
+
+ Added two regression tests to ensure WebKit doesn't erroneously remove inline styles of an element
+ when its implicit styles differ from the inline styles. Also rebaselined some tests as their results
+ have progressed.
+
+ * editing/pasteboard/insert-u-with-text-decoration-none-expected.txt: Added.
+ * editing/pasteboard/insert-u-with-text-decoration-none.html: Added.
+ * editing/pasteboard/insert-font-with-size-and-css-expected.txt: Added.
+ * editing/pasteboard/insert-font-with-size-and-css.html: Added.
+ * editing/pasteboard/4744008-expected.txt:
+ * editing/pasteboard/paste-text-with-style-5-expected.txt:
+ * editing/pasteboard/paste-text-with-style-5.html:
+ * editing/pasteboard/style-from-rules-expected.txt:
+ * editing/pasteboard/style-from-rules.html:
+
2013-02-22 Nils Barth <[email protected]>
REGRESSION(r130089): Scrollbar thumb no longer re-rendered on hover
Modified: trunk/LayoutTests/editing/pasteboard/4744008-expected.txt (143820 => 143821)
--- trunk/LayoutTests/editing/pasteboard/4744008-expected.txt 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/LayoutTests/editing/pasteboard/4744008-expected.txt 2013-02-23 01:54:30 UTC (rev 143821)
@@ -1,7 +1,7 @@
EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document
EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 3 of #text > DIV > BODY > HTML > #document to 3 of #text > 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 7 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
This tests for a crash on paste inside removeRedundantStyles. You should see 'foo bar' below.
Added: trunk/LayoutTests/editing/pasteboard/insert-font-with-size-and-css-expected.txt (0 => 143821)
--- trunk/LayoutTests/editing/pasteboard/insert-font-with-size-and-css-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/insert-font-with-size-and-css-expected.txt 2013-02-23 01:54:30 UTC (rev 143821)
@@ -0,0 +1,30 @@
+This tests inserting a font element with both font-size CSS property and size attribute.
+WebKit should not strip just font-size property. Two instances of "hello world" should look identical to each other.
+
+Insertion point:
+| <div>
+| contenteditable=""
+| style="font-size: 25px;"
+| "WebKit"
+
+Content to insert:
+| <font>
+| size="7"
+| style="font-size: 25px;"
+| "hello "
+| <font>
+| color="blue"
+| size="7"
+| style="font-size: 15px;"
+| "world "
+
+After insertion:
+| <div>
+| contenteditable=""
+| style="font-size: 25px;"
+| "hello "
+| <font>
+| color="blue"
+| style="font-size: 15px;"
+| "world <#selection-caret>"
+| "WebKit"
Added: trunk/LayoutTests/editing/pasteboard/insert-font-with-size-and-css.html (0 => 143821)
--- trunk/LayoutTests/editing/pasteboard/insert-font-with-size-and-css.html (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/insert-font-with-size-and-css.html 2013-02-23 01:54:30 UTC (rev 143821)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="container"><div contenteditable style="font-size: 25px;">WebKit</div></div>
+<div id="content"><font size="7" style="font-size: 25px;">hello </font><font size="7" color="blue" style="font-size: 15px;">world </font></div>
+<script src=""
+<script>
+
+Markup.description('This tests inserting a font element with both font-size CSS property and size attribute.\n'
+ + 'WebKit should not strip just font-size property. Two instances of "hello world" should look identical to each other.');
+
+Markup.dump('container', 'Insertion point');
+
+var content = document.getElementById('content');
+Markup.dump(content, 'Content to insert');
+
+document.querySelector('div[contenteditable]').focus();
+document.execCommand('InsertHTML', null, content.innerHTML);
+
+Markup.dump('container', 'After insertion');
+
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/editing/pasteboard/insert-u-with-text-decoration-none-expected.txt (0 => 143821)
--- trunk/LayoutTests/editing/pasteboard/insert-u-with-text-decoration-none-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/insert-u-with-text-decoration-none-expected.txt 2013-02-23 01:54:30 UTC (rev 143821)
@@ -0,0 +1,24 @@
+This tests inserting an u element with text-decoration property set to none.
+WebKit should not strip just font-size property. Two instances of "hello world" should look identical to each other.
+
+Insertion point:
+| <div>
+| contenteditable=""
+| " WebKit"
+
+Content to insert:
+| <u>
+| style="text-decoration: none;"
+| "hello "
+| <u>
+| style="text-decoration: line-through;"
+| "world"
+
+After insertion:
+| <div>
+| contenteditable=""
+| "hello "
+| <span>
+| style="text-decoration: line-through;"
+| "world<#selection-caret>"
+| " WebKit"
Added: trunk/LayoutTests/editing/pasteboard/insert-u-with-text-decoration-none.html (0 => 143821)
--- trunk/LayoutTests/editing/pasteboard/insert-u-with-text-decoration-none.html (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/insert-u-with-text-decoration-none.html 2013-02-23 01:54:30 UTC (rev 143821)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="container"><div contenteditable> WebKit</div></div>
+<div id="content"><u style="text-decoration: none;">hello </u><u style="text-decoration: line-through;">world</u></div>
+<script src=""
+<script>
+
+Markup.description('This tests inserting an u element with text-decoration property set to none.\n'
+ + 'WebKit should not strip just font-size property. Two instances of "hello world" should look identical to each other.');
+
+Markup.dump('container', 'Insertion point');
+
+var content = document.getElementById('content');
+Markup.dump(content, 'Content to insert');
+
+document.querySelector('div[contenteditable]').focus();
+document.execCommand('InsertHTML', null, content.innerHTML);
+
+Markup.dump('container', 'After insertion');
+
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/editing/pasteboard/paste-text-with-style-5-expected.txt (143820 => 143821)
--- trunk/LayoutTests/editing/pasteboard/paste-text-with-style-5-expected.txt 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/LayoutTests/editing/pasteboard/paste-text-with-style-5-expected.txt 2013-02-23 01:54:30 UTC (rev 143821)
@@ -1,5 +1,5 @@
This tests copying and pasting text does not strip inline styles that overrides UA style rules.
-To manually test, copy and paste "hello world" below. The pasted text should be bolded.
+To manually test, copy and paste "hello world" below. The pasted text should not be bolded.
Before copy-paste:
| <b>
@@ -7,7 +7,5 @@
| "<#selection-caret>hello world"
After copy-paste:
-| <b>
-| style="font-weight: normal;"
-| "hello world<#selection-caret>"
+| "hello world<#selection-caret>"
| <br>
Modified: trunk/LayoutTests/editing/pasteboard/paste-text-with-style-5.html (143820 => 143821)
--- trunk/LayoutTests/editing/pasteboard/paste-text-with-style-5.html 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/LayoutTests/editing/pasteboard/paste-text-with-style-5.html 2013-02-23 01:54:30 UTC (rev 143821)
@@ -2,7 +2,7 @@
<html>
<body>
<p id="description">This tests copying and pasting text does not strip inline styles that overrides UA style rules.
-To manually test, copy and paste "hello world" below. The pasted text should be bolded.</p>
+To manually test, copy and paste "hello world" below. The pasted text should not be bolded.</p>
<div id="test" contenteditable><b style="font-weight: normal">hello world</b></div>
<script src=""
<script>
Modified: trunk/LayoutTests/editing/pasteboard/style-from-rules-expected.txt (143820 => 143821)
--- trunk/LayoutTests/editing/pasteboard/style-from-rules-expected.txt 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/LayoutTests/editing/pasteboard/style-from-rules-expected.txt 2013-02-23 01:54:30 UTC (rev 143821)
@@ -19,7 +19,7 @@
"
| <em>
| style="color: blue;"
-| title="font-style: normal; font-weight: bold; color: blue;"
+| title="font-weight: bold; color: blue;"
| "WebKit"
Pasted:
@@ -34,7 +34,7 @@
| title="font-size: 1em; font-weight: bold;"
| "world"
| " "
-| <em>
-| style="font-style: normal; font-weight: bold; color: blue;"
-| title="font-style: normal; font-weight: bold; color: blue;"
+| <span>
+| style="font-weight: bold; color: blue;"
+| title="font-weight: bold; color: blue;"
| "WebKit<#selection-caret>"
Modified: trunk/LayoutTests/editing/pasteboard/style-from-rules.html (143820 => 143821)
--- trunk/LayoutTests/editing/pasteboard/style-from-rules.html 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/LayoutTests/editing/pasteboard/style-from-rules.html 2013-02-23 01:54:30 UTC (rev 143821)
@@ -16,7 +16,7 @@
<section id="container">
<div id="copy" contenteditable><blockquote title="none">hello</blockquote>
<p title="none"><span class="red" style="font-size: 1em; font-weight: bold;" title="font-size: 1em; font-weight: bold;">world</span>
-<em style="color: blue;" title="font-style: normal; font-weight: bold; color: blue;">WebKit</em></p></div>
+<em style="color: blue;" title="font-weight: bold; color: blue;">WebKit</em></p></div>
<div id="paste" contenteditable></div>
</section>
<script src=""
Modified: trunk/Source/WebCore/ChangeLog (143820 => 143821)
--- trunk/Source/WebCore/ChangeLog 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/Source/WebCore/ChangeLog 2013-02-23 01:54:30 UTC (rev 143821)
@@ -1,3 +1,37 @@
+2013-02-22 Ryosuke Niwa <[email protected]>
+
+ WebKit can erroneously strip font-size CSS property from font element with size attribute
+ https://bugs.webkit.org/show_bug.cgi?id=110657
+
+ Reviewed by Justin Garcia.
+
+ The bug was caused by ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline
+ erroneously removing style attributes even on an element such as font that implicitly adds
+ editing style.
+
+ Fixed the bug by removing these elements or attributes when they conflict with the inline
+ style of the element. This is always safe because implicit style of an element is always
+ overridden by that of inline style.
+
+ Note that when the font element becomes "empty" (i.e. doesn't have any attributes), then
+ we also want to delete this font element as it doesn't contribute anything to the style.
+
+ Test: editing/pasteboard/insert-u-with-text-decoration-none.html
+ editing/pasteboard/insert-font-with-size-and-css.html
+
+ * editing/ApplyStyleCommand.cpp:
+ (WebCore::isEmptyFontTag): Added ShouldStyleAttributeBeEmpty as an argument. This will
+ allow removeRedundantStylesAndKeepStyleSpanInline to ignore style attribute when we know
+ the attribute can be removed.
+
+ * editing/ApplyStyleCommand.h: Expose isEmptyFontTag and ShouldStyleAttributeBeEmpty.
+
+ * editing/ReplaceSelectionCommand.cpp:
+ (WebCore::ReplaceSelectionCommand::InsertedNodes::didReplaceNode): Added.
+ (WebCore::ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline): See above.
+ * editing/ReplaceSelectionCommand.h:
+ (InsertedNodes): Added a declaration of didReplaceNode.
+
2013-02-22 Laszlo Gombos <[email protected]>
Remove unused make variable from DerivedSources.make
Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.cpp (143820 => 143821)
--- trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2013-02-23 01:54:30 UTC (rev 143821)
@@ -71,7 +71,6 @@
return elem->hasLocalName(spanAttr) && elem->getAttribute(classAttr) == styleSpanClassString();
}
-enum ShouldStyleAttributeBeEmpty { AllowNonEmptyStyleAttribute, StyleAttributeShouldBeEmpty };
static bool hasNoAttributeOrOnlyStyleAttribute(const StyledElement* element, ShouldStyleAttributeBeEmpty shouldStyleAttributeBeEmpty)
{
if (!element->hasAttributes())
@@ -102,15 +101,12 @@
return hasNoAttributeOrOnlyStyleAttribute(toHTMLElement(node), StyleAttributeShouldBeEmpty);
}
-static bool isEmptyFontTag(const Node *node)
+bool isEmptyFontTag(const Element* element, ShouldStyleAttributeBeEmpty shouldStyleAttributeBeEmpty)
{
- if (!node || !node->hasTagName(fontTag))
+ if (!element || !element->hasTagName(fontTag))
return false;
- const Element *elem = static_cast<const Element *>(node);
- if (!elem->hasAttributes())
- return true;
- return elem->attributeCount() == 1 && elem->getAttribute(classAttr) == styleSpanClassString();
+ return hasNoAttributeOrOnlyStyleAttribute(static_cast<const HTMLElement*>(element), shouldStyleAttributeBeEmpty);
}
static PassRefPtr<Element> createFontElement(Document* document)
Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.h (143820 => 143821)
--- trunk/Source/WebCore/editing/ApplyStyleCommand.h 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/Source/WebCore/editing/ApplyStyleCommand.h 2013-02-23 01:54:30 UTC (rev 143821)
@@ -132,6 +132,8 @@
IsInlineElementToRemoveFunction m_isInlineElementToRemoveFunction;
};
+enum ShouldStyleAttributeBeEmpty { AllowNonEmptyStyleAttribute, StyleAttributeShouldBeEmpty };
+bool isEmptyFontTag(const Element*, ShouldStyleAttributeBeEmpty = StyleAttributeShouldBeEmpty);
bool isLegacyAppleStyleSpan(const Node*);
bool isStyleSpanOrSpanWithOnlyStyleAttribute(const Element*);
PassRefPtr<HTMLElement> createStyleSpanElement(Document*);
Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp (143820 => 143821)
--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2013-02-23 01:54:30 UTC (rev 143821)
@@ -356,6 +356,14 @@
m_lastNodeInserted = NodeTraversal::previousSkippingChildren(m_lastNodeInserted.get());
}
+inline void ReplaceSelectionCommand::InsertedNodes::didReplaceNode(Node* node, Node* newNode)
+{
+ if (m_firstNodeInserted == node)
+ m_firstNodeInserted = newNode;
+ if (m_lastNodeInserted == node)
+ m_lastNodeInserted = newNode;
+}
+
ReplaceSelectionCommand::ReplaceSelectionCommand(Document* document, PassRefPtr<DocumentFragment> fragment, CommandOptions options, EditAction editAction)
: CompositeEditCommand(document)
, m_selectReplacement(options & SelectReplacement)
@@ -476,6 +484,23 @@
const StylePropertySet* inlineStyle = element->inlineStyle();
RefPtr<EditingStyle> newInlineStyle = EditingStyle::create(inlineStyle);
if (inlineStyle) {
+ if (element->isHTMLElement()) {
+ Vector<QualifiedName> attributes;
+ HTMLElement* htmlElement = static_cast<HTMLElement*>(element);
+
+ if (newInlineStyle->conflictsWithImplicitStyleOfElement(htmlElement)) {
+ // e.g. <b style="font-weight: normal;"> is converted to <span style="font-weight: normal;">
+ node = replaceElementWithSpanPreservingChildrenAndAttributes(htmlElement);
+ element = static_cast<StyledElement*>(node.get());
+ insertedNodes.didReplaceNode(htmlElement, node.get());
+ } else if (newInlineStyle->extractConflictingImplicitStyleOfAttributes(htmlElement, EditingStyle::PreserveWritingDirection, 0, attributes,
+ EditingStyle::DoNotExtractMatchingStyle)) {
+ // e.g. <font size="3" style="font-size: 20px;"> is converted to <font style="font-size: 20px;">
+ for (size_t i = 0; i < attributes.size(); i++)
+ removeNodeAttribute(element, attributes[i]);
+ }
+ }
+
ContainerNode* context = element->parentNode();
// If Mail wraps the fragment with a Paste as Quotation blockquote, or if you're pasting into a quoted region,
@@ -488,12 +513,12 @@
}
if (!inlineStyle || newInlineStyle->isEmpty()) {
- if (isStyleSpanOrSpanWithOnlyStyleAttribute(element)) {
+ if (isStyleSpanOrSpanWithOnlyStyleAttribute(element) || isEmptyFontTag(element, AllowNonEmptyStyleAttribute)) {
insertedNodes.willRemoveNodePreservingChildren(element);
removeNodePreservingChildren(element);
continue;
- } else
- removeNodeAttribute(element, styleAttr);
+ }
+ removeNodeAttribute(element, styleAttr);
} else if (newInlineStyle->style()->propertyCount() != inlineStyle->propertyCount())
setNodeAttribute(element, styleAttr, newInlineStyle->style()->asText());
Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.h (143820 => 143821)
--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.h 2013-02-23 01:32:31 UTC (rev 143820)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.h 2013-02-23 01:54:30 UTC (rev 143821)
@@ -63,6 +63,7 @@
void respondToNodeInsertion(Node*);
void willRemoveNodePreservingChildren(Node*);
void willRemoveNode(Node*);
+ void didReplaceNode(Node*, Node* newNode);
Node* firstNodeInserted() const { return m_firstNodeInserted.get(); }
Node* lastLeafInserted() const { return m_lastNodeInserted->lastDescendant(); }
