Changeset 65208 in webkit


Ignore:
Timestamp:
Aug 11, 2010 7:05:11 PM (14 years ago)
Author:
rniwa@webkit.org
Message:

2010-08-11 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

Can't unbold text in div in font-weight span
​https://bugs.webkit.org/show_bug.cgi?id=26871

The bug was caused by removeInlineStyle not being able to include styled inline nodes around the start.
Solved this problem by pushing down all inline styles instead of just text-decorations.
This approach allows removeInlineStyle to remove styled ancestors properly and generates compact markups.

Test: editing/style/push-down-inline-styles.html

  • editing/ApplyStyleCommand.cpp: (WebCore::ApplyStyleCommand::removeCSSStyle): No longer removes attributes or node when mode == RemoveNone. (WebCore::ApplyStyleCommand::highestAncestorWithConflictingInlineStyle): Calls shouldRemoveInlineStyleFromElement to determine the highest ancestor whose style needs to be pushed down. (WebCore::ApplyStyleCommand::extractInlineStyleToPushDown): Renamed from extractTextDecorationStyle. Extracts all inline CSS properties specified instead of just text decorations. (WebCore::ApplyStyleCommand::applyInlineStyleToPushDown): Renamed from applyTextDecorationStyle. Applies inline styles using addInlineStyleIfNeeded or adds inline CSS values. (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): Renamed from pushDownTextDecorationStyleAroundNode. (WebCore::ApplyStyleCommand::removeInlineStyle): Calls pushDownTextDecorationStyleAroundNode.
  • editing/ApplyStyleCommand.h:
  • editing/DeleteSelectionCommand.cpp: (WebCore::DeleteSelectionCommand::mergeParagraphs): Prevents moveParagraph from preserving the style of an empty paragraph when merged with the previous paragraph because we don't use that style anyways.

2010-08-11 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

Can't unbold text in div in font-weight span
​https://bugs.webkit.org/show_bug.cgi?id=26871

Added a test to push down inline styles to the leaves of DOM tree.

  • editing/deleting/delete-br-011-expected.txt: Removed redundant style spans.
  • editing/execCommand/empty-span-removal-expected.txt: Removed a span without any attributes.
  • editing/style/push-down-inline-styles-expected.txt: Added.
  • editing/style/push-down-inline-styles.html: Added.
  • editing/style/script-tests/push-down-inline-styles.js: Added. (testSingleToggle):
Location:
trunk
Files:
3 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r65206 r65208  
     12010-08-11  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        Can't unbold text in div in font-weight span
     6        https://bugs.webkit.org/show_bug.cgi?id=26871
     7
     8        Added a test to push down inline styles to the leaves of DOM tree.
     9
     10        * editing/deleting/delete-br-011-expected.txt: Removed redundant style spans.
     11        * editing/execCommand/empty-span-removal-expected.txt: Removed a span without any attributes.
     12        * editing/style/push-down-inline-styles-expected.txt: Added.
     13        * editing/style/push-down-inline-styles.html: Added.
     14        * editing/style/script-tests/push-down-inline-styles.js: Added.
     15        (testSingleToggle):
     16
    1172010-08-11  Shinichiro Hamaji  <hamaji@chromium.org>
    218

  • trunk/LayoutTests/editing/deleting/delete-br-011-expected.txt

    r64808 r65208  
    2727|   class="editing"
    2828|   id="test"
    29 |   <span>
    30 |     class="Apple-style-span"
    31 |     style="font-size: medium;"
    32 |     <font>
    33 |       class="Apple-style-span"
    34 |       size="6"
    35 |       <span>
    36 |         class="Apple-style-span"
    37 |         style="font-size: 24px;"
    38 |         <#selection-caret>
    39 |         <br>
     29|   <#selection-caret>
     30|   <br>
    4031| "
    4132"

  • trunk/LayoutTests/editing/execCommand/empty-span-removal-expected.txt

    r40139 r65208  
    55
    66PASS one bold command converted <span><span style='font-weight: bold'>test</span></span> to <span>test</span>
    7 PASS one bold command converted <span style='font-weight: bold'><span>test</span></span> to <span>test</span>
     7PASS one bold command converted <span style='font-weight: bold'><span>test</span></span> to test
    88PASS one bold command converted <span style='font-weight: bold'><span style='font-weight: bold'>test</span></span> to test
    99PASS one bold command converted <span foo="bar" style='font-weight: bold'>test</span> to <span foo="bar">test</span>

  • trunk/LayoutTests/editing/execCommand/script-tests/empty-span-removal.js

    r48548 r65208  
    3131
    3232testSingleToggle("bold", "<span><span style='font-weight: bold'>test</span></span>", "<span>test</span>");
    33 testSingleToggle("bold", "<span style='font-weight: bold'><span>test</span></span>", "<span>test</span>");
     33testSingleToggle("bold", "<span style='font-weight: bold'><span>test</span></span>", "test");
    3434testSingleToggle("bold", "<span style='font-weight: bold'><span style='font-weight: bold'>test</span></span>", "test");
    3535testSingleToggle("bold", "<span foo=\"bar\" style='font-weight: bold'>test</span>", "<span foo=\"bar\">test</span>");

  • trunk/WebCore/ChangeLog

    r65205 r65208  
     12010-08-11  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        Can't unbold text in div in font-weight span
     6        https://bugs.webkit.org/show_bug.cgi?id=26871
     7
     8        The bug was caused by removeInlineStyle not being able to include styled inline nodes around the start.
     9        Solved this problem by pushing down all inline styles instead of just text-decorations.
     10        This approach allows removeInlineStyle to remove styled ancestors properly and generates compact markups.
     11
     12        Test: editing/style/push-down-inline-styles.html
     13
     14        * editing/ApplyStyleCommand.cpp:
     15        (WebCore::ApplyStyleCommand::removeCSSStyle): No longer removes attributes or node when mode == RemoveNone.
     16        (WebCore::ApplyStyleCommand::highestAncestorWithConflictingInlineStyle): Calls shouldRemoveInlineStyleFromElement
     17        to determine the highest ancestor whose style needs to be pushed down.
     18        (WebCore::ApplyStyleCommand::extractInlineStyleToPushDown): Renamed from extractTextDecorationStyle.
     19        Extracts all inline CSS properties specified instead of just text decorations.
     20        (WebCore::ApplyStyleCommand::applyInlineStyleToPushDown): Renamed from applyTextDecorationStyle.
     21        Applies inline styles using addInlineStyleIfNeeded or adds inline CSS values.
     22        (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): Renamed from pushDownTextDecorationStyleAroundNode.
     23        (WebCore::ApplyStyleCommand::removeInlineStyle): Calls pushDownTextDecorationStyleAroundNode.
     24        * editing/ApplyStyleCommand.h:
     25        * editing/DeleteSelectionCommand.cpp:
     26        (WebCore::DeleteSelectionCommand::mergeParagraphs): Prevents moveParagraph from preserving
     27        the style of an empty paragraph when merged with the previous paragraph because we don't use that style anyways.
     28
    1292010-08-11  Julien Chaffraix  <jchaffraix@codeaurora.org>
    230

  • trunk/WebCore/editing/ApplyStyleCommand.cpp

    r65019 r65208  
    13121312    }
    13131313
     1314    if (mode == RemoveNone)
     1315        return removed;
     1316
    13141317    // No need to serialize <foo style=""> if we just removed the last css property
    13151318    if (decl->isEmpty())
    … …  
    13221325}
    13231326
    1324 static bool hasTextDecorationProperty(Node *node)
    1325 {
    1326     if (!node->isElementNode())
    1327         return false;
    1328 
    1329     RefPtr<CSSValue> value = computedStyle(node)->getPropertyCSSValue(CSSPropertyTextDecoration, DoNotUpdateLayout);
    1330     return value && !equalIgnoringCase(value->cssText(), "none");
    1331 }
    1332 
    1333 static Node* highestAncestorWithTextDecoration(Node *node)
    1334 {
    1335     ASSERT(node);
    1336     Node* result = 0;
     1327HTMLElement* ApplyStyleCommand::highestAncestorWithConflictingInlineStyle(CSSMutableStyleDeclaration* style, Node* node)
     1328{
     1329    if (!node)
     1330        return 0;
     1331
     1332    HTMLElement* result = 0;
    13371333    Node* unsplittableElement = unsplittableElementForPosition(Position(node, 0));
    13381334
    13391335    for (Node *n = node; n; n = n->parentNode()) {
    1340         if (hasTextDecorationProperty(n))
    1341             result = n;
     1336        if (n->isHTMLElement() && shouldRemoveInlineStyleFromElement(style, static_cast<HTMLElement*>(n)))
     1337            result = static_cast<HTMLElement*>(n);
    13421338        // Should stop at the editable root (cannot cross editing boundary) and
    13431339        // also stop at the unsplittable element to be consistent with other UAs
    … …  
    13491345}
    13501346
    1351 PassRefPtr<CSSMutableStyleDeclaration> ApplyStyleCommand::extractTextDecorationStyle(Node* node)
     1347PassRefPtr<CSSMutableStyleDeclaration> ApplyStyleCommand::extractInlineStyleToPushDown(Node* node, const Vector<int>& properties)
    13521348{
    13531349    ASSERT(node);
    … …  
    13631359        return 0;
    13641360
    1365     int properties[1] = { CSSPropertyTextDecoration };
    1366     RefPtr<CSSMutableStyleDeclaration> textDecorationStyle = style->copyPropertiesInSet(properties, 1);
    1367 
    1368     RefPtr<CSSValue> property = style->getPropertyCSSValue(CSSPropertyTextDecoration);
    1369     if (property && !equalIgnoringCase(property->cssText(), "none"))
    1370         removeCSSProperty(element, CSSPropertyTextDecoration);
    1371 
    1372     return textDecorationStyle.release();
    1373 }
    1374 
    1375 void ApplyStyleCommand::applyTextDecorationStyle(Node *node, CSSMutableStyleDeclaration *style)
     1361    style = style->copyPropertiesInSet(properties.data(), properties.size());
     1362
     1363    for (size_t i = 0; i < properties.size(); i++) {
     1364        RefPtr<CSSValue> property = style->getPropertyCSSValue(properties[i]);
     1365        if (property)
     1366            removeCSSProperty(element, static_cast<CSSPropertyID>(properties[i]));
     1367    }
     1368
     1369    if (element->inlineStyleDecl() && element->inlineStyleDecl()->isEmpty())
     1370        removeNodeAttribute(element, styleAttr);
     1371
     1372    if (isSpanWithoutAttributesOrUnstyleStyleSpan(element))
     1373        removeNodePreservingChildren(element);
     1374
     1375    return style.release();
     1376}
     1377
     1378void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, CSSMutableStyleDeclaration* style)
    13761379{
    13771380    ASSERT(node);
    13781381
    1379     if (!style || style->cssText().isEmpty())
     1382    if (!style || !style->length() || !node->renderer())
    13801383        return;
    13811384
    1382     StyleChange styleChange(style, Position(node, 0));
    1383     if (styleChange.cssStyle().length()) {
    1384         if (node->isTextNode()) {
    1385             RefPtr<HTMLElement> styleSpan = createStyleSpanElement(document());
    1386             surroundNodeRangeWithElement(node, node, styleSpan.get());
    1387             node = styleSpan.get();
    1388         }
    1389 
    1390         if (!node->isElementNode())
    1391             return;
    1392 
    1393         HTMLElement *element = static_cast<HTMLElement *>(node);
    1394         String cssText = styleChange.cssStyle();
    1395         CSSMutableStyleDeclaration *decl = element->inlineStyleDecl();
    1396         if (decl)
    1397             cssText += decl->cssText();
    1398         setNodeAttribute(element, styleAttr, cssText);
    1399     }
    1400 
    1401     if (styleChange.applyUnderline())
    1402         surroundNodeRangeWithElement(node, node, createHTMLElement(document(), uTag));
    1403 
    1404     if (styleChange.applyLineThrough())
    1405         surroundNodeRangeWithElement(node, node, createHTMLElement(document(), sTag));   
    1406 }
    1407 
    1408 void ApplyStyleCommand::pushDownTextDecorationStyleAroundNode(Node* targetNode)
    1409 {
    1410     ASSERT(targetNode);
    1411     Node* highestAncestor = highestAncestorWithTextDecoration(targetNode);
     1385    // Since addInlineStyleIfNeeded can't add styles to block-flow render objects, add style attribute instead.
     1386    // FIXME: applyInlineStyleToRange should be used here instead.
     1387    if ((node->renderer()->isBlockFlow() || node->childNodeCount()) && node->isHTMLElement()) {
     1388        HTMLElement* element = static_cast<HTMLElement*>(node);
     1389        CSSMutableStyleDeclaration* existingInlineStyle = element->inlineStyleDecl();
     1390
     1391        // Avoid overriding existing styles of node
     1392        if (existingInlineStyle) {
     1393            RefPtr<CSSMutableStyleDeclaration> newInlineStyle = existingInlineStyle->copy();
     1394            CSSMutableStyleDeclaration::const_iterator end = style->end();
     1395            for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
     1396                ExceptionCode ec;
     1397                if (!existingInlineStyle->getPropertyCSSValue(it->id()))
     1398                    newInlineStyle->setProperty(it->id(), it->value()->cssText(), it->isImportant(), ec);
     1399
     1400                // text-decorations adds up
     1401                if (it->id() == CSSPropertyTextDecoration) {
     1402                    ASSERT(it->value()->isValueList());
     1403                    RefPtr<CSSValue> textDecoration = newInlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration);
     1404                    if (textDecoration) {
     1405                        ASSERT(textDecoration->isValueList());
     1406                        CSSValueList* textDecorationOfInlineStyle = static_cast<CSSValueList*>(textDecoration.get());
     1407                        CSSValueList* textDecorationOfStyleApplied = static_cast<CSSValueList*>(it->value());
     1408
     1409                        DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, underline, (CSSPrimitiveValue::createIdentifier(CSSValueUnderline)));
     1410                        DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, lineThrough, (CSSPrimitiveValue::createIdentifier(CSSValueLineThrough)));
     1411                       
     1412                        if (textDecorationOfStyleApplied->hasValue(underline.get()) && !textDecorationOfInlineStyle->hasValue(underline.get()))
     1413                            textDecorationOfInlineStyle->append(underline.get());
     1414
     1415                        if (textDecorationOfStyleApplied->hasValue(lineThrough.get()) && !textDecorationOfInlineStyle->hasValue(lineThrough.get()))
     1416                            textDecorationOfInlineStyle->append(lineThrough.get());
     1417                    }
     1418                }
     1419            }
     1420
     1421            setNodeAttribute(element, styleAttr, newInlineStyle->cssText());
     1422        } else
     1423            setNodeAttribute(element, styleAttr, style->cssText());
     1424
     1425        return;
     1426    }
     1427
     1428    if (node->renderer()->isText() && static_cast<RenderText*>(node->renderer())->isAllCollapsibleWhitespace())
     1429        return;
     1430
     1431    // FIXME: addInlineStyleIfNeeded may override the style of node
     1432    addInlineStyleIfNeeded(style, node, node);
     1433}
     1434
     1435void ApplyStyleCommand::pushDownInlineStyleAroundNode(CSSMutableStyleDeclaration* style, Node* targetNode)
     1436{
     1437    HTMLElement* highestAncestor = highestAncestorWithConflictingInlineStyle(style, targetNode);
    14121438    if (!highestAncestor)
    14131439        return;
     1440
     1441    Vector<int> properties;
     1442    CSSMutableStyleDeclaration::const_iterator end = style->end();
     1443    for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it)
     1444        properties.append(it->id());
    14141445
    14151446    // The outer loop is traversing the tree vertically from highestAncestor to targetNode
    … …  
    14171448    while (current != targetNode) {
    14181449        ASSERT(current);
     1450        ASSERT(current->isHTMLElement());
    14191451        ASSERT(current->contains(targetNode));
    1420         RefPtr<CSSMutableStyleDeclaration> decoration = extractTextDecorationStyle(current);
     1452        Node* child = current->firstChild();
     1453        RefPtr<CSSMutableStyleDeclaration> styleToPushDown = extractInlineStyleToPushDown(current, properties);
    14211454
    14221455        // The inner loop will go through children on each level
    1423         Node* child = current->firstChild();
    14241456        while (child) {
    14251457            Node* nextChild = child->nextSibling();
    … …  
    14271459            // Apply text decoration to all nodes containing targetNode and their siblings but NOT to targetNode
    14281460            if (child != targetNode)
    1429                 applyTextDecorationStyle(child, decoration.get());
    1430            
     1461                applyInlineStyleToPushDown(child, styleToPushDown.get());
     1462
    14311463            // We found the next node for the outer loop (contains targetNode)
    14321464            // When reached targetNode, stop the outer loop upon the completion of the current inner loop
    … …  
    14481480   
    14491481    RefPtr<CSSValue> textDecorationSpecialProperty = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
    1450 
    14511482    if (textDecorationSpecialProperty) {
    1452         pushDownTextDecorationStyleAroundNode(start.downstream().node());
    1453         pushDownTextDecorationStyleAroundNode(end.upstream().node());
    14541483        style = style->copy();
    14551484        style->setProperty(CSSPropertyTextDecoration, textDecorationSpecialProperty->cssText(), style->getPropertyPriority(CSSPropertyWebkitTextDecorationsInEffect));
    14561485    }
     1486
     1487    RefPtr<Node> pushDownEnd = end.upstream().node();
     1488    pushDownInlineStyleAroundNode(style.get(), start.downstream().node());
     1489    pushDownInlineStyleAroundNode(style.get(), pushDownEnd.get());
    14571490
    14581491    // The s and e variables store the positions used to set the ending selection after style removal

  • trunk/WebCore/editing/ApplyStyleCommand.h

    r65013 r65208  
    7979    bool removeHTMLBidiEmbeddingStyle(CSSMutableStyleDeclaration*, HTMLElement*, InlineStyleRemovalMode = RemoveAttributesAndElements);
    8080    bool removeCSSStyle(CSSMutableStyleDeclaration*, HTMLElement*, InlineStyleRemovalMode = RemoveAttributesAndElements);
     81    HTMLElement* highestAncestorWithConflictingInlineStyle(CSSMutableStyleDeclaration*, Node*);
     82    PassRefPtr<CSSMutableStyleDeclaration> extractInlineStyleToPushDown(Node*, const Vector<int>&);
     83    void applyInlineStyleToPushDown(Node*, CSSMutableStyleDeclaration *style);
     84    void pushDownInlineStyleAroundNode(CSSMutableStyleDeclaration*, Node*);
    8185    void removeInlineStyle(PassRefPtr<CSSMutableStyleDeclaration>, const Position& start, const Position& end);
    8286    bool nodeFullySelected(Node*, const Position& start, const Position& end) const;
    8387    bool nodeFullyUnselected(Node*, const Position& start, const Position& end) const;
    84     PassRefPtr<CSSMutableStyleDeclaration> extractTextDecorationStyle(Node*);
    85     void applyTextDecorationStyle(Node*, CSSMutableStyleDeclaration *style);
    86     void pushDownTextDecorationStyleAroundNode(Node*);
    8788
    8889    // style-application helpers

  • trunk/WebCore/editing/DeleteSelectionCommand.cpp

    r63773 r65208  
    631631    // removals that it does cause the insertion of *another* placeholder.
    632632    bool needPlaceholder = m_needPlaceholder;
    633     moveParagraph(startOfParagraphToMove, endOfParagraphToMove, mergeDestination);
     633    bool paragraphToMergeIsEmpty = (startOfParagraphToMove == endOfParagraphToMove);
     634    moveParagraph(startOfParagraphToMove, endOfParagraphToMove, mergeDestination, false, !paragraphToMergeIsEmpty);
    634635    m_needPlaceholder = needPlaceholder;
    635636    // The endingPosition was likely clobbered by the move, so recompute it (moveParagraph selects the moved paragraph).
Note: See TracChangeset for help on using the changeset viewer.