Commit b249876e authored by rniwa@webkit.org's avatar rniwa@webkit.org

2011-03-01 Ryosuke Niwa <rniwa@webkit.org>

        Reviewed by Darin Adler.

        WebKit does not merge text decorations in the typing style and the selected element properly
        https://bugs.webkit.org/show_bug.cgi?id=55349

        Added a test to ensure WebKit merges text decorations in the typing style and the inline style
        of the element around the caret when computing the style at the selection start.

        * editing/execCommand/merge-text-decoration-with-typing-style-expected.txt: Added.
        * editing/execCommand/merge-text-decoration-with-typing-style.html: Added.
        * editing/style/push-down-inline-styles-expected.txt: Rebaselined due to the change in which text decoration
        values appear.
        * editing/style/script-tests/push-down-inline-styles.js: Ditto.
2011-03-01  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Darin Adler.

        WebKit does not merge text decorations in the typing style and the selected element properly
        https://bugs.webkit.org/show_bug.cgi?id=55349

        The bug was caused by EditingStyle::mergeTypingStyle's not properly merging text decoration property.
        Fixed the bug by extracting a function from ApplyStyleCommand::pushDownInlineStyleAroundNode and
        calling it in pushDownInlineStyleAroundNode and in mergeTypingStyle.

        Test: editing/execCommand/merge-text-decoration-with-typing-style.html

        * editing/ApplyStyleCommand.cpp:
        (WebCore::ApplyStyleCommand::applyInlineStyleToPushDown): Takes EditingStyle*;
        calls mergeInlineStyleOfElement.
        (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): Calls applyInlineStyleToPushDown.
        (WebCore::ApplyStyleCommand::removeInlineStyle): Ditto.
        * editing/ApplyStyleCommand.h:
        * editing/EditingStyle.cpp:
        (WebCore::EditingStyle::mergeTypingStyle): Added; calls mergeStyle.
        (WebCore::EditingStyle::mergeInlineStyleOfElement): Ditto.
        (WebCore::EditingStyle::mergeStyle): Extracted from applyInlineStyleToPushDown.
        * editing/EditingStyle.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@80060 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ca40abff
2011-03-01 Ryosuke Niwa <rniwa@webkit.org>
Reviewed by Darin Adler.
WebKit does not merge text decorations in the typing style and the selected element properly
https://bugs.webkit.org/show_bug.cgi?id=55349
Added a test to ensure WebKit merges text decorations in the typing style and the inline style
of the element around the caret when computing the style at the selection start.
* editing/execCommand/merge-text-decoration-with-typing-style-expected.txt: Added.
* editing/execCommand/merge-text-decoration-with-typing-style.html: Added.
* editing/style/push-down-inline-styles-expected.txt: Rebaselined due to the change in which text decoration
values appear.
* editing/style/script-tests/push-down-inline-styles.js: Ditto.
2011-03-01 Tony Gentilcore <tonyg@chromium.org>
Unreviewed expectations update
......
This test ensures WebKit merges text decorations in the typing style and the focused element's style property.
hello world
Test to set strikeThrough by typing command and add underline as inline style:
PASS
Test to set underline by typing command and add line-through as inline style:
PASS
<!DOCTYPE html>
<html>
<body>
<p>This test ensures WebKit merges text decorations in the typing style and the focused element's style property.</p>
<div id="test" contenteditable>hello world</div>
<pre><script>
if (window.layoutTestController)
layoutTestController.dumpAsText();
function queryTextDecorations() {
var result = '';
if (document.queryCommandState('underline'))
result += 'underline';
if (document.queryCommandState('strikeThrough')) {
if (result.length)
result += ' ';
result += 'strikeThrough';
}
return result;
}
function runTest(command, inlineStyle) {
document.writeln('Test to set ' + command + ' by typing command and add ' + inlineStyle + ' as inline style:');
var test = document.getElementById('test');
test.style.textDecoration = null;
window.getSelection().setPosition(test, 0);
document.execCommand(command, false, null);
test.style.textDecoration = inlineStyle;
if (queryTextDecorations() != 'underline strikeThrough')
document.writeln('FAIL: got "' + queryTextDecorations() + '" but expected "underline strikeThrough"');
else
document.writeln('PASS');
document.writeln();
}
runTest('strikeThrough', 'underline');
window.getSelection().setPosition(test, 1);
runTest('underline', 'line-through');
</script></pre>
</body>
</html>
......@@ -19,9 +19,9 @@ PASS italic converted <span style="font-style: italic;">hello <span id="test">wo
PASS underline converted <span style="text-decoration: underline;"><div id="test">hello</div>world</span> to <div id="test">hello</div><u>world</u>
PASS underline converted <span style="text-decoration: underline;"><div id="test">hello</div><blockquote>world<br>webkit</blockquote></span> to <div id="test">hello</div><blockquote style="text-decoration: underline; ">world<br>webkit</blockquote>
PASS underline converted <span style="text-decoration: underline;">hello<div id="test">world</div>webkit</u> to <u>hello</u><div id="test">world</div><u>webkit</u>
FAIL underline converted <div style="text-decoration: underline;"><div>hello</span></div><div id="test">webkit</div><span style="font-style: italic;">rocks</span> to <div><div style="text-decoration: underline; ">hello</div><div id="test">webkit</div><span style="font-style: italic; text-decoration: underline; ">rocks</span></div>, expected <div><div style="text-decoration: underline; ">hello</span></div><div id="test">webkit</div><u><span style="font-style: italic;">rocks</span></u></div>
PASS underline converted <span style="text-decoration: underline;"><div style="text-decoration: line-through;">hello</div><div id="test">world</div></span> to <div style="text-decoration: line-through underline; ">hello</div><div id="test">world</div>
PASS strikeThrough converted <span style="text-decoration: line-through;"><div id="test">hello</div><div style="text-decoration: underline;">world</div></span> to <div id="test">hello</div><div style="text-decoration: underline line-through; ">world</div>
FAIL underline converted <div style="text-decoration: underline;"><div>hello</span></div><div id="test">webkit</div><span style="font-style: italic;">rocks</span> to <div><div style="text-decoration: underline; ">hello</div><div id="test">webkit</div><span style="text-decoration: underline; font-style: italic; ">rocks</span></div>, expected <div><div style="text-decoration: underline; ">hello</span></div><div id="test">webkit</div><u><span style="font-style: italic;">rocks</span></u></div>
PASS underline converted <span style="text-decoration: underline;"><div style="text-decoration: line-through;">hello</div><div id="test">world</div></span> to <div style="text-decoration: underline line-through; ">hello</div><div id="test">world</div>
PASS strikeThrough converted <span style="text-decoration: line-through;"><div id="test">hello</div><div style="text-decoration: underline;">world</div></span> to <div id="test">hello</div><div style="text-decoration: line-through underline; ">world</div>
PASS successfullyParsed is true
TEST COMPLETE
......
......@@ -40,8 +40,8 @@ testSingleToggle("underline", '<span style="text-decoration: underline;">hello<d
testSingleToggle("underline",
'<div style="text-decoration: underline;"><div>hello</span></div><div id="test">webkit</div><span style="font-style: italic;">rocks</span>',
'<div><div style="text-decoration: underline; ">hello</span></div><div id="test">webkit</div><u><span style="font-style: italic;">rocks</span></u></div>');
testSingleToggle("underline", '<span style="text-decoration: underline;"><div style="text-decoration: line-through;">hello</div><div id="test">world</div></span>', '<div style="text-decoration: line-through underline; ">hello</div><div id="test">world</div>');
testSingleToggle("strikeThrough", '<span style="text-decoration: line-through;"><div id="test">hello</div><div style="text-decoration: underline;">world</div></span>', '<div id="test">hello</div><div style="text-decoration: underline line-through; ">world</div>');
testSingleToggle("underline", '<span style="text-decoration: underline;"><div style="text-decoration: line-through;">hello</div><div id="test">world</div></span>', '<div style="text-decoration: underline line-through; ">hello</div><div id="test">world</div>');
testSingleToggle("strikeThrough", '<span style="text-decoration: line-through;"><div id="test">hello</div><div style="text-decoration: underline;">world</div></span>', '<div id="test">hello</div><div style="text-decoration: line-through underline; ">world</div>');
document.body.removeChild(testContainer);
......
2011-03-01 Ryosuke Niwa <rniwa@webkit.org>
Reviewed by Darin Adler.
WebKit does not merge text decorations in the typing style and the selected element properly
https://bugs.webkit.org/show_bug.cgi?id=55349
The bug was caused by EditingStyle::mergeTypingStyle's not properly merging text decoration property.
Fixed the bug by extracting a function from ApplyStyleCommand::pushDownInlineStyleAroundNode and
calling it in pushDownInlineStyleAroundNode and in mergeTypingStyle.
Test: editing/execCommand/merge-text-decoration-with-typing-style.html
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::applyInlineStyleToPushDown): Takes EditingStyle*;
calls mergeInlineStyleOfElement.
(WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): Calls applyInlineStyleToPushDown.
(WebCore::ApplyStyleCommand::removeInlineStyle): Ditto.
* editing/ApplyStyleCommand.h:
* editing/EditingStyle.cpp:
(WebCore::EditingStyle::mergeTypingStyle): Added; calls mergeStyle.
(WebCore::EditingStyle::mergeInlineStyleOfElement): Ditto.
(WebCore::EditingStyle::mergeStyle): Extracted from applyInlineStyleToPushDown.
* editing/EditingStyle.h:
2011-03-01 Levi Weintraub <leviw@chromium.org>
Reviewed by Ryosuke Niwa.
......
......@@ -1270,52 +1270,23 @@ HTMLElement* ApplyStyleCommand::highestAncestorWithConflictingInlineStyle(Editin
return result;
}
void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, CSSMutableStyleDeclaration* style)
void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, EditingStyle* style)
{
ASSERT(node);
if (!style || !style->length() || !node->renderer())
if (!style || style->isEmpty() || !node->renderer())
return;
RefPtr<CSSMutableStyleDeclaration> newInlineStyle = style;
if (node->isHTMLElement()) {
HTMLElement* element = toHTMLElement(node);
CSSMutableStyleDeclaration* existingInlineStyle = element->inlineStyleDecl();
// Avoid overriding existing styles of node
if (existingInlineStyle) {
newInlineStyle = existingInlineStyle->copy();
CSSMutableStyleDeclaration::const_iterator end = style->end();
for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
ExceptionCode ec;
if (!existingInlineStyle->getPropertyCSSValue(it->id()))
newInlineStyle->setProperty(it->id(), it->value()->cssText(), it->isImportant(), ec);
// text-decorations adds up
if (it->id() == CSSPropertyTextDecoration && it->value()->isValueList()) {
RefPtr<CSSValue> textDecoration = newInlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration);
if (textDecoration && textDecoration->isValueList()) {
CSSValueList* textDecorationOfInlineStyle = static_cast<CSSValueList*>(textDecoration.get());
CSSValueList* textDecorationOfStyleApplied = static_cast<CSSValueList*>(it->value());
DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, underline, (CSSPrimitiveValue::createIdentifier(CSSValueUnderline)));
DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, lineThrough, (CSSPrimitiveValue::createIdentifier(CSSValueLineThrough)));
if (textDecorationOfStyleApplied->hasValue(underline.get()) && !textDecorationOfInlineStyle->hasValue(underline.get()))
textDecorationOfInlineStyle->append(underline.get());
if (textDecorationOfStyleApplied->hasValue(lineThrough.get()) && !textDecorationOfInlineStyle->hasValue(lineThrough.get()))
textDecorationOfInlineStyle->append(lineThrough.get());
}
}
}
}
RefPtr<EditingStyle> newInlineStyle = style;
if (node->isHTMLElement() && static_cast<HTMLElement*>(node)->inlineStyleDecl()) {
newInlineStyle = style->copy();
newInlineStyle->mergeInlineStyleOfElement(static_cast<HTMLElement*>(node));
}
// Since addInlineStyleIfNeeded can't add styles to block-flow render objects, add style attribute instead.
// FIXME: applyInlineStyleToRange should be used here instead.
if ((node->renderer()->isBlockFlow() || node->childNodeCount()) && node->isHTMLElement()) {
setNodeAttribute(toHTMLElement(node), styleAttr, newInlineStyle->cssText());
setNodeAttribute(toHTMLElement(node), styleAttr, newInlineStyle->style()->cssText());
return;
}
......@@ -1325,7 +1296,7 @@ void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, CSSMutableStyleDe
// We can't wrap node with the styled element here because new styled element will never be removed if we did.
// If we modified the child pointer in pushDownInlineStyleAroundNode to point to new style element
// then we fall into an infinite loop where we keep removing and adding styled element wrapping node.
addInlineStyleIfNeeded(newInlineStyle.get(), node, node, DoNotAddStyledElement);
addInlineStyleIfNeeded(newInlineStyle->style(), node, node, DoNotAddStyledElement);
}
void ApplyStyleCommand::pushDownInlineStyleAroundNode(EditingStyle* style, Node* targetNode)
......@@ -1371,7 +1342,7 @@ void ApplyStyleCommand::pushDownInlineStyleAroundNode(EditingStyle* style, Node*
// Apply text decoration to all nodes containing targetNode and their siblings but NOT to targetNode
// But if we've removed styledElement then go ahead and always apply the style.
if (child != targetNode || styledElement)
applyInlineStyleToPushDown(child, styleToPushDown->style());
applyInlineStyleToPushDown(child, styleToPushDown.get());
// We found the next node for the outer loop (contains targetNode)
// When reached targetNode, stop the outer loop upon the completion of the current inner loop
......@@ -1448,7 +1419,7 @@ void ApplyStyleCommand::removeInlineStyle(EditingStyle* style, const Position &s
if (styleToPushDown) {
for (; childNode; childNode = childNode->nextSibling())
applyInlineStyleToPushDown(childNode.get(), styleToPushDown->style());
applyInlineStyleToPushDown(childNode.get(), styleToPushDown.get());
}
}
if (node == end.deprecatedNode())
......
......@@ -84,7 +84,7 @@ private:
bool removeImplicitlyStyledElement(EditingStyle*, HTMLElement*, InlineStyleRemovalMode, EditingStyle* extractedStyle);
bool removeCSSStyle(EditingStyle*, HTMLElement*, InlineStyleRemovalMode = RemoveIfNeeded, EditingStyle* extractedStyle = 0);
HTMLElement* highestAncestorWithConflictingInlineStyle(EditingStyle*, Node*);
void applyInlineStyleToPushDown(Node*, CSSMutableStyleDeclaration*);
void applyInlineStyleToPushDown(Node*, EditingStyle*);
void pushDownInlineStyleAroundNode(EditingStyle*, Node*);
void removeInlineStyle(EditingStyle* , const Position& start, const Position& end);
bool nodeFullySelected(Node*, const Position& start, const Position& end) const;
......
......@@ -685,13 +685,55 @@ void EditingStyle::mergeTypingStyle(Document* document)
ASSERT(document);
RefPtr<EditingStyle> typingStyle = document->frame()->selection()->typingStyle();
if (!typingStyle || !typingStyle->style() || typingStyle == this)
if (!typingStyle || typingStyle == this)
return;
if (m_mutableStyle)
m_mutableStyle->merge(typingStyle->style(), true);
else
m_mutableStyle = typingStyle->style()->copy();
mergeStyle(typingStyle->style());
}
void EditingStyle::mergeInlineStyleOfElement(StyledElement* element)
{
ASSERT(element);
mergeStyle(element->inlineStyleDecl());
}
void EditingStyle::mergeStyle(CSSMutableStyleDeclaration* style)
{
if (!style)
return;
if (!m_mutableStyle) {
m_mutableStyle = style->copy();
return;
}
CSSMutableStyleDeclaration::const_iterator end = style->end();
for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
RefPtr<CSSValue> value;
if ((it->id() == CSSPropertyTextDecoration || it->id() == CSSPropertyWebkitTextDecorationsInEffect) && it->value()->isValueList()) {
value = m_mutableStyle->getPropertyCSSValue(it->id());
if (value && !value->isValueList())
value = 0;
}
if (!value) {
ExceptionCode ec;
m_mutableStyle->setProperty(it->id(), it->value()->cssText(), it->isImportant(), ec);
continue;
}
CSSValueList* newTextDecorations = static_cast<CSSValueList*>(it->value());
CSSValueList* textDecorations = static_cast<CSSValueList*>(value.get());
DEFINE_STATIC_LOCAL(const RefPtr<CSSPrimitiveValue>, underline, (CSSPrimitiveValue::createIdentifier(CSSValueUnderline)));
DEFINE_STATIC_LOCAL(const RefPtr<CSSPrimitiveValue>, lineThrough, (CSSPrimitiveValue::createIdentifier(CSSValueLineThrough)));
if (newTextDecorations->hasValue(underline.get()) && !textDecorations->hasValue(underline.get()))
textDecorations->append(underline.get());
if (newTextDecorations->hasValue(lineThrough.get()) && !textDecorations->hasValue(lineThrough.get()))
textDecorations->append(lineThrough.get());
}
}
}
......@@ -106,6 +106,7 @@ public:
Vector<QualifiedName>& conflictingAttributes, ShouldExtractMatchingStyle) const;
void prepareToApplyAt(const Position&, ShouldPreserveWritingDirection = DoNotPreserveWritingDirection);
void mergeTypingStyle(Document*);
void mergeInlineStyleOfElement(StyledElement*);
float fontSizeDelta() const { return m_fontSizeDelta; }
bool hasFontSizeDelta() const { return m_fontSizeDelta != NoFontDelta; }
......@@ -122,6 +123,7 @@ private:
void replaceFontSizeByKeywordIfPossible(RenderStyle*, CSSComputedStyleDeclaration*);
void extractFontSizeDelta();
bool conflictsWithInlineStyleOfElement(StyledElement*, EditingStyle* extractedStyle, Vector<CSSPropertyID>* conflictingProperties) const;
void mergeStyle(CSSMutableStyleDeclaration*);
RefPtr<CSSMutableStyleDeclaration> m_mutableStyle;
bool m_shouldUseFixedDefaultFontSize;
......
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