Changeset 80060 in webkit


Ignore:
Timestamp:
Mar 1, 2011 4:14:34 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

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:
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r80058 r80060  
     12011-03-01  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        WebKit does not merge text decorations in the typing style and the selected element properly
     6        https://bugs.webkit.org/show_bug.cgi?id=55349
     7
     8        Added a test to ensure WebKit merges text decorations in the typing style and the inline style
     9        of the element around the caret when computing the style at the selection start.
     10
     11        * editing/execCommand/merge-text-decoration-with-typing-style-expected.txt: Added.
     12        * editing/execCommand/merge-text-decoration-with-typing-style.html: Added.
     13        * editing/style/push-down-inline-styles-expected.txt: Rebaselined due to the change in which text decoration
     14        values appear.
     15        * editing/style/script-tests/push-down-inline-styles.js: Ditto.
     16
    1172011-03-01  Tony Gentilcore  <tonyg@chromium.org>
    218

  • trunk/LayoutTests/editing/style/push-down-inline-styles-expected.txt

    r65208 r80060  
    2020PASS 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>
    2121PASS 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>
    22 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>
    23 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>
    24 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>
     22FAIL 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>
     23PASS 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>
     24PASS 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>
    2525PASS successfullyParsed is true
    2626

  • trunk/LayoutTests/editing/style/script-tests/push-down-inline-styles.js

    r65208 r80060  
    4141    '<div style="text-decoration: underline;"><div>hello</span></div><div id="test">webkit</div><span style="font-style: italic;">rocks</span>',
    4242    '<div><div style="text-decoration: underline; ">hello</span></div><div id="test">webkit</div><u><span style="font-style: italic;">rocks</span></u></div>');
    43 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>');
    44 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>');
     43testSingleToggle("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>');
     44testSingleToggle("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>');
    4545
    4646document.body.removeChild(testContainer);

  • trunk/Source/WebCore/ChangeLog

    r80059 r80060  
     12011-03-01  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        WebKit does not merge text decorations in the typing style and the selected element properly
     6        https://bugs.webkit.org/show_bug.cgi?id=55349
     7
     8        The bug was caused by EditingStyle::mergeTypingStyle's not properly merging text decoration property.
     9        Fixed the bug by extracting a function from ApplyStyleCommand::pushDownInlineStyleAroundNode and
     10        calling it in pushDownInlineStyleAroundNode and in mergeTypingStyle.
     11
     12        Test: editing/execCommand/merge-text-decoration-with-typing-style.html
     13
     14        * editing/ApplyStyleCommand.cpp:
     15        (WebCore::ApplyStyleCommand::applyInlineStyleToPushDown): Takes EditingStyle*;
     16        calls mergeInlineStyleOfElement.
     17        (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): Calls applyInlineStyleToPushDown.
     18        (WebCore::ApplyStyleCommand::removeInlineStyle): Ditto.
     19        * editing/ApplyStyleCommand.h:
     20        * editing/EditingStyle.cpp:
     21        (WebCore::EditingStyle::mergeTypingStyle): Added; calls mergeStyle.
     22        (WebCore::EditingStyle::mergeInlineStyleOfElement): Ditto.
     23        (WebCore::EditingStyle::mergeStyle): Extracted from applyInlineStyleToPushDown.
     24        * editing/EditingStyle.h:
     25
    1262011-03-01  Levi Weintraub  <leviw@chromium.org>
    227

  • trunk/Source/WebCore/editing/ApplyStyleCommand.cpp

    r79976 r80060  
    12711271}
    12721272
    1273 void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, CSSMutableStyleDeclaration* style)
     1273void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, EditingStyle* style)
    12741274{
    12751275    ASSERT(node);
    12761276
    1277     if (!style || !style->length() || !node->renderer())
     1277    if (!style || style->isEmpty() || !node->renderer())
    12781278        return;
    12791279
    1280     RefPtr<CSSMutableStyleDeclaration> newInlineStyle = style;
    1281     if (node->isHTMLElement()) {
    1282         HTMLElement* element = toHTMLElement(node);
    1283         CSSMutableStyleDeclaration* existingInlineStyle = element->inlineStyleDecl();
    1284 
    1285         // Avoid overriding existing styles of node
    1286         if (existingInlineStyle) {
    1287             newInlineStyle = existingInlineStyle->copy();
    1288             CSSMutableStyleDeclaration::const_iterator end = style->end();
    1289             for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
    1290                 ExceptionCode ec;
    1291                 if (!existingInlineStyle->getPropertyCSSValue(it->id()))
    1292                     newInlineStyle->setProperty(it->id(), it->value()->cssText(), it->isImportant(), ec);
    1293 
    1294                 // text-decorations adds up
    1295                 if (it->id() == CSSPropertyTextDecoration && it->value()->isValueList()) {
    1296                     RefPtr<CSSValue> textDecoration = newInlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration);
    1297                     if (textDecoration && textDecoration->isValueList()) {
    1298                         CSSValueList* textDecorationOfInlineStyle = static_cast<CSSValueList*>(textDecoration.get());
    1299                         CSSValueList* textDecorationOfStyleApplied = static_cast<CSSValueList*>(it->value());
    1300 
    1301                         DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, underline, (CSSPrimitiveValue::createIdentifier(CSSValueUnderline)));
    1302                         DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, lineThrough, (CSSPrimitiveValue::createIdentifier(CSSValueLineThrough)));
    1303                        
    1304                         if (textDecorationOfStyleApplied->hasValue(underline.get()) && !textDecorationOfInlineStyle->hasValue(underline.get()))
    1305                             textDecorationOfInlineStyle->append(underline.get());
    1306 
    1307                         if (textDecorationOfStyleApplied->hasValue(lineThrough.get()) && !textDecorationOfInlineStyle->hasValue(lineThrough.get()))
    1308                             textDecorationOfInlineStyle->append(lineThrough.get());
    1309                     }
    1310                 }
    1311             }
    1312         }
     1280    RefPtr<EditingStyle> newInlineStyle = style;
     1281    if (node->isHTMLElement() && static_cast<HTMLElement*>(node)->inlineStyleDecl()) {
     1282        newInlineStyle = style->copy();
     1283        newInlineStyle->mergeInlineStyleOfElement(static_cast<HTMLElement*>(node));
    13131284    }
    13141285
    … …  
    13161287    // FIXME: applyInlineStyleToRange should be used here instead.
    13171288    if ((node->renderer()->isBlockFlow() || node->childNodeCount()) && node->isHTMLElement()) {
    1318         setNodeAttribute(toHTMLElement(node), styleAttr, newInlineStyle->cssText());
     1289        setNodeAttribute(toHTMLElement(node), styleAttr, newInlineStyle->style()->cssText());
    13191290        return;
    13201291    }
    … …  
    13261297    // If we modified the child pointer in pushDownInlineStyleAroundNode to point to new style element
    13271298    // then we fall into an infinite loop where we keep removing and adding styled element wrapping node.
    1328     addInlineStyleIfNeeded(newInlineStyle.get(), node, node, DoNotAddStyledElement);
     1299    addInlineStyleIfNeeded(newInlineStyle->style(), node, node, DoNotAddStyledElement);
    13291300}
    13301301
    … …  
    13721343            // But if we've removed styledElement then go ahead and always apply the style.
    13731344            if (child != targetNode || styledElement)
    1374                 applyInlineStyleToPushDown(child, styleToPushDown->style());
     1345                applyInlineStyleToPushDown(child, styleToPushDown.get());
    13751346
    13761347            // We found the next node for the outer loop (contains targetNode)
    … …  
    14491420            if (styleToPushDown) {
    14501421                for (; childNode; childNode = childNode->nextSibling())
    1451                     applyInlineStyleToPushDown(childNode.get(), styleToPushDown->style());
     1422                    applyInlineStyleToPushDown(childNode.get(), styleToPushDown.get());
    14521423            }
    14531424        }

  • trunk/Source/WebCore/editing/ApplyStyleCommand.h

    r79976 r80060  
    8585    bool removeCSSStyle(EditingStyle*, HTMLElement*, InlineStyleRemovalMode = RemoveIfNeeded, EditingStyle* extractedStyle = 0);
    8686    HTMLElement* highestAncestorWithConflictingInlineStyle(EditingStyle*, Node*);
    87     void applyInlineStyleToPushDown(Node*, CSSMutableStyleDeclaration*);
     87    void applyInlineStyleToPushDown(Node*, EditingStyle*);
    8888    void pushDownInlineStyleAroundNode(EditingStyle*, Node*);
    8989    void removeInlineStyle(EditingStyle* , const Position& start, const Position& end);

  • trunk/Source/WebCore/editing/EditingStyle.cpp

    r79976 r80060  
    686686
    687687    RefPtr<EditingStyle> typingStyle = document->frame()->selection()->typingStyle();
    688     if (!typingStyle || !typingStyle->style() || typingStyle == this)
    689         return;
    690 
    691     if (m_mutableStyle)
    692         m_mutableStyle->merge(typingStyle->style(), true);
    693     else
    694         m_mutableStyle = typingStyle->style()->copy();
    695 }
    696    
    697 }
     688    if (!typingStyle || typingStyle == this)
     689        return;
     690
     691    mergeStyle(typingStyle->style());
     692}
     693
     694void EditingStyle::mergeInlineStyleOfElement(StyledElement* element)
     695{
     696    ASSERT(element);
     697    mergeStyle(element->inlineStyleDecl());
     698}
     699
     700void EditingStyle::mergeStyle(CSSMutableStyleDeclaration* style)
     701{
     702    if (!style)
     703        return;
     704
     705    if (!m_mutableStyle) {
     706        m_mutableStyle = style->copy();
     707        return;
     708    }
     709
     710    CSSMutableStyleDeclaration::const_iterator end = style->end();
     711    for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
     712        RefPtr<CSSValue> value;
     713        if ((it->id() == CSSPropertyTextDecoration || it->id() == CSSPropertyWebkitTextDecorationsInEffect) && it->value()->isValueList()) {
     714            value = m_mutableStyle->getPropertyCSSValue(it->id());
     715            if (value && !value->isValueList())
     716                value = 0;
     717        }
     718
     719        if (!value) {
     720            ExceptionCode ec;
     721            m_mutableStyle->setProperty(it->id(), it->value()->cssText(), it->isImportant(), ec);
     722            continue;
     723        }
     724
     725        CSSValueList* newTextDecorations = static_cast<CSSValueList*>(it->value());
     726        CSSValueList* textDecorations = static_cast<CSSValueList*>(value.get());
     727
     728        DEFINE_STATIC_LOCAL(const RefPtr<CSSPrimitiveValue>, underline, (CSSPrimitiveValue::createIdentifier(CSSValueUnderline)));
     729        DEFINE_STATIC_LOCAL(const RefPtr<CSSPrimitiveValue>, lineThrough, (CSSPrimitiveValue::createIdentifier(CSSValueLineThrough)));
     730
     731        if (newTextDecorations->hasValue(underline.get()) && !textDecorations->hasValue(underline.get()))
     732            textDecorations->append(underline.get());
     733
     734        if (newTextDecorations->hasValue(lineThrough.get()) && !textDecorations->hasValue(lineThrough.get()))
     735            textDecorations->append(lineThrough.get());
     736    }
     737}
     738
     739}

  • trunk/Source/WebCore/editing/EditingStyle.h

    r79976 r80060  
    107107    void prepareToApplyAt(const Position&, ShouldPreserveWritingDirection = DoNotPreserveWritingDirection);
    108108    void mergeTypingStyle(Document*);
     109    void mergeInlineStyleOfElement(StyledElement*);
    109110
    110111    float fontSizeDelta() const { return m_fontSizeDelta; }
    … …  
    123124    void extractFontSizeDelta();
    124125    bool conflictsWithInlineStyleOfElement(StyledElement*, EditingStyle* extractedStyle, Vector<CSSPropertyID>* conflictingProperties) const;
     126    void mergeStyle(CSSMutableStyleDeclaration*);
    125127
    126128    RefPtr<CSSMutableStyleDeclaration> m_mutableStyle;
Note: See TracChangeset for help on using the changeset viewer.