Changeset 69910 in webkit


Ignore:
Timestamp:
Oct 15, 2010 9:10:26 PM (14 years ago)
Author:
rniwa@webkit.org
Message:

2010-10-15 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

unlink removes inline style of anchor elements
​https://bugs.webkit.org/show_bug.cgi?id=47424

The bug was caused by our not extracting styles when the anchor element is removed by removeInlineStyle.
Because we removed the element without pushing its inline style down, we lost style information.

Fixed the bug by pushing down styles in removeInlineStyle as done in pushDownInlineStyleAroundNode.
Also fixed a bug in addInlineStyleIfNeeded where StyleChange incorrectly removed styles of the container
node as supposed to that of startNode from the style to apply when startNode is not an element.

Test: editing/execCommand/toggle-unlink.html

  • editing/ApplyStyleCommand.cpp: (WebCore::ApplyStyleCommand::removeInlineStyleFromElement): To prevent a similar bug in the future, added an assert that extractedStyle is specified whenever we're removing a styled element. (WebCore::ApplyStyleCommand::removeInlineStyle): (WebCore::ApplyStyleCommand::addInlineStyleIfNeeded):

2010-10-15 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

unlink removes inline style of anchor elements
​https://bugs.webkit.org/show_bug.cgi?id=47424

Added a test to ensure execCommand('unlink', false, null) preserves the inline style declaration.

  • editing/execCommand/script-tests/toggle-unlink.js: Added. (testSingleToggle): (selectAll): (selectFirstTwoWords): (selectLastTwoWords): (selectLastWord):
  • editing/execCommand/toggle-unlink-expected.txt: Added.
  • editing/execCommand/toggle-unlink.html: Added.
  • editing/style/script-tests/make-text-writing-direction-inline.js: Specifies both direction and unicode-bidi properties instead of just direction property.
Location:
trunk
Files:
3 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r69907 r69910  
     12010-10-15  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        unlink removes inline style of anchor elements
     6        https://bugs.webkit.org/show_bug.cgi?id=47424
     7
     8        Added a test to ensure execCommand('unlink', false, null) preserves the inline style declaration.
     9
     10        * editing/execCommand/script-tests/toggle-unlink.js: Added.
     11        (testSingleToggle):
     12        (selectAll):
     13        (selectFirstTwoWords):
     14        (selectLastTwoWords):
     15        (selectLastWord):
     16        * editing/execCommand/toggle-unlink-expected.txt: Added.
     17        * editing/execCommand/toggle-unlink.html: Added.
     18        * editing/style/script-tests/make-text-writing-direction-inline.js: Specifies both direction
     19        and unicode-bidi properties instead of just direction property.
     20
    1212010-10-15  Kinuko Yasuda  <kinuko@chromium.org>
    222

  • trunk/LayoutTests/editing/style/script-tests/make-text-writing-direction-inline.js

    r67490 r69910  
    9898                       '<div dir="rtl">هنا <span dir="ltr">يكتب</span> النص<span dir="ltr"> العربي</span></div>');
    9999modifyWritingDirection('<div dir="rtl">هنا <span dir="ltr">يكتب النص العربي</span></div>', selectThirdWord, 'LeftToRight',
    100                        '<div dir="rtl"><span style="direction: ltr;">هنا يكتب النص العربي</span></div>');
     100                       '<div dir="rtl"><span style="unicode-bidi: embed; direction: ltr;">هنا يكتب النص العربي</span></div>');
    101101modifyWritingDirection('<div dir="rtl">هنا <span dir="ltr">يكتب النص العربي</span></div>', selectThirdWord, 'RightToLeft',
    102102                       '<div dir="rtl">هنا <span dir="ltr">يكتب</span><span style="unicode-bidi: embed;"> النص</span><span dir="ltr"> العربي</span></div>');

  • trunk/WebCore/ChangeLog

    r69909 r69910  
     12010-10-15  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        unlink removes inline style of anchor elements
     6        https://bugs.webkit.org/show_bug.cgi?id=47424
     7
     8        The bug was caused by our not extracting styles when the anchor element is removed by removeInlineStyle.
     9        Because we removed the element without pushing its inline style down, we lost style information.
     10
     11        Fixed the bug by pushing down styles in removeInlineStyle as done in pushDownInlineStyleAroundNode.
     12        Also fixed a bug in addInlineStyleIfNeeded where StyleChange incorrectly removed styles of the container
     13        node as supposed to that of startNode from the style to apply when startNode is not an element.
     14
     15        Test: editing/execCommand/toggle-unlink.html
     16
     17        * editing/ApplyStyleCommand.cpp:
     18        (WebCore::ApplyStyleCommand::removeInlineStyleFromElement): To prevent a similar bug in the future,
     19        added an assert that extractedStyle is specified whenever we're removing a styled element.
     20        (WebCore::ApplyStyleCommand::removeInlineStyle):
     21        (WebCore::ApplyStyleCommand::addInlineStyleIfNeeded):
     22
    1232010-10-15  Ryosuke Niwa  <rniwa@webkit.org>
    224

  • trunk/WebCore/editing/ApplyStyleCommand.cpp

    r69868 r69910  
    12241224
    12251225    if (m_styledInlineElement && element->hasTagName(m_styledInlineElement->tagQName())) {
    1226         if (mode != RemoveNone) {
    1227             if (extractedStyle && element->inlineStyleDecl())
    1228                 extractedStyle->merge(element->inlineStyleDecl());
    1229             removeNodePreservingChildren(element);
    1230         }
     1226        if (mode == RemoveNone)
     1227            return true;
     1228        ASSERT(extractedStyle);
     1229        if (element->inlineStyleDecl())
     1230            extractedStyle->merge(element->inlineStyleDecl());
     1231        removeNodePreservingChildren(element);
    12311232        return true;
    12321233    }
    … …  
    15871588    Node* node = start.node();
    15881589    while (node) {
    1589         Node* next = node->traverseNextNode();
     1590        RefPtr<Node> next = node->traverseNextNode();
    15901591        if (node->isHTMLElement() && nodeFullySelected(node, start, end)) {
    1591             HTMLElement* elem = static_cast<HTMLElement*>(node);
    1592             Node* prev = elem->traversePreviousNodePostOrder();
    1593             Node* next = elem->traverseNextNode();
    1594             removeInlineStyleFromElement(style.get(), elem);
     1592            RefPtr<HTMLElement> elem = static_cast<HTMLElement*>(node);
     1593            RefPtr<Node> prev = elem->traversePreviousNodePostOrder();
     1594            RefPtr<Node> next = elem->traverseNextNode();
     1595            RefPtr<CSSMutableStyleDeclaration> styleToPushDown;
     1596            PassRefPtr<Node> childNode = 0;
     1597            if (m_styledInlineElement && elem->hasTagName(m_styledInlineElement->tagQName())) {
     1598                styleToPushDown = CSSMutableStyleDeclaration::create();
     1599                childNode = elem->firstChild();
     1600            }
     1601
     1602            removeInlineStyleFromElement(style.get(), elem.get(), RemoveIfNeeded, styleToPushDown.get());
    15951603            if (!elem->inDocument()) {
    15961604                if (s.node() == elem) {
    … …  
    16051613                    // the max range offset of prev.
    16061614                    ASSERT(e.deprecatedEditingOffset() >= lastOffsetForEditing(e.node()));
    1607                     e = Position(prev, lastOffsetForEditing(prev));
     1615                    e = Position(prev, lastOffsetForEditing(prev.get()));
    16081616                }
    16091617            }
     1618
     1619            if (styleToPushDown) {
     1620                for (; childNode; childNode = childNode->nextSibling())
     1621                    applyInlineStyleToPushDown(childNode.get(), styleToPushDown.get());
     1622            }
    16101623        }
    16111624        if (node == end.node())
    16121625            break;
    1613         node = next;
     1626        node = next.get();
    16141627    }
    16151628   
    … …  
    18681881{
    18691882    // It's okay to obtain the style at the startNode because we've removed all relevant styles from the current run.
    1870     StyleChange styleChange(style, Position(startNode, 0));
     1883    RefPtr<HTMLElement> dummyElement;
     1884    Position positionForStyleComparison;
     1885    if (!startNode->isElementNode()) {
     1886        dummyElement = createStyleSpanElement(document());
     1887        insertNodeAt(dummyElement, positionBeforeNode(startNode));
     1888        positionForStyleComparison = positionBeforeNode(dummyElement.get());
     1889    } else
     1890        positionForStyleComparison = firstPositionInNode(startNode);
     1891
     1892    StyleChange styleChange(style, positionForStyleComparison);
     1893
     1894    if (dummyElement)
     1895        removeNode(dummyElement);
    18711896
    18721897    // Find appropriate font and span elements top-down.
Note: See TracChangeset for help on using the changeset viewer.