Changeset 44435 in webkit


Ignore:
Timestamp:
Jun 4, 2009 3:17:09 PM (15 years ago)
Author:
eric@webkit.org
Message:

2009-02-03 Eric Seidel <​eric@webkit.org>

Reviewed by Justin Garcia.

Make sure execCommand("bold") on <b style="text-decoration: underline">test</b>
only removes the bold and not the underline.
​https://bugs.webkit.org/show_bug.cgi?id=23496

Test: editing/execCommand/convert-style-elements-to-spans.html

  • WebCore.xcodeproj/project.pbxproj:
  • css/CSSStyleDeclaration.h: (WebCore::CSSStyleDeclaration::isEmpty):
  • dom/NamedAttrMap.h: (WebCore::NamedAttrMap::isEmpty):
  • editing/ApplyStyleCommand.cpp: (WebCore::isUnstyledStyleSpan): (WebCore::isSpanWithoutAttributesOrUnstyleStyleSpan): (WebCore::ApplyStyleCommand::applyBlockStyle): (WebCore::ApplyStyleCommand::applyRelativeFontStyleChange): (WebCore::ApplyStyleCommand::implicitlyStyledElementShouldBeRemovedWhenApplyingStyle): (WebCore::ApplyStyleCommand::replaceWithSpanOrRemoveIfWithoutAttributes): (WebCore::ApplyStyleCommand::removeCSSStyle): (WebCore::ApplyStyleCommand::applyTextDecorationStyle): (WebCore::ApplyStyleCommand::removeInlineStyle): (WebCore::ApplyStyleCommand::addInlineStyleIfNeeded):
  • editing/ApplyStyleCommand.h:
  • editing/CompositeEditCommand.cpp: (WebCore::CompositeEditCommand::replaceNodeWithSpanPreservingChildrenAndAttributes):
  • editing/CompositeEditCommand.h:
  • editing/RemoveNodePreservingChildrenCommand.cpp: (WebCore::RemoveNodePreservingChildrenCommand::RemoveNodePreservingChildrenCommand):
  • editing/ReplaceNodeWithSpanCommand.cpp: Added. (WebCore::ReplaceNodeWithSpanCommand::ReplaceNodeWithSpanCommand): (WebCore::swapInNodePreservingAttributesAndChildren): (WebCore::ReplaceNodeWithSpanCommand::doApply): (WebCore::ReplaceNodeWithSpanCommand::doUnapply):
  • editing/ReplaceNodeWithSpanCommand.h: Added. (WebCore::ReplaceNodeWithSpanCommand::create):
Location:
trunk
Files:
5 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r44429 r44435  
     12009-02-03  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Justin Garcia.
     4
     5        Test to make sure execCommand("bold") on <b style="text-decoration: underline">test</b>
     6        only removes the bold and not the underline.
     7        https://bugs.webkit.org/show_bug.cgi?id=23496
     8
     9        * editing/execCommand/convert-style-elements-to-spans-expected.txt: Added.
     10        * editing/execCommand/convert-style-elements-to-spans.html: Added.
     11        * editing/execCommand/resources/convert-style-elements-to-spans.js: Added.
     12        (testSingleToggle):
     13        (testDoubleToggle):
     14
    1152009-06-04  David Hyatt  <hyatt@apple.com>
    216

  • trunk/WebCore/ChangeLog

    r44434 r44435  
     12009-02-03  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Justin Garcia.
     4
     5        Make sure execCommand("bold") on <b style="text-decoration: underline">test</b>
     6        only removes the bold and not the underline.
     7        https://bugs.webkit.org/show_bug.cgi?id=23496
     8
     9        Test: editing/execCommand/convert-style-elements-to-spans.html
     10
     11        * WebCore.xcodeproj/project.pbxproj:
     12        * css/CSSStyleDeclaration.h:
     13        (WebCore::CSSStyleDeclaration::isEmpty):
     14        * dom/NamedAttrMap.h:
     15        (WebCore::NamedAttrMap::isEmpty):
     16        * editing/ApplyStyleCommand.cpp:
     17        (WebCore::isUnstyledStyleSpan):
     18        (WebCore::isSpanWithoutAttributesOrUnstyleStyleSpan):
     19        (WebCore::ApplyStyleCommand::applyBlockStyle):
     20        (WebCore::ApplyStyleCommand::applyRelativeFontStyleChange):
     21        (WebCore::ApplyStyleCommand::implicitlyStyledElementShouldBeRemovedWhenApplyingStyle):
     22        (WebCore::ApplyStyleCommand::replaceWithSpanOrRemoveIfWithoutAttributes):
     23        (WebCore::ApplyStyleCommand::removeCSSStyle):
     24        (WebCore::ApplyStyleCommand::applyTextDecorationStyle):
     25        (WebCore::ApplyStyleCommand::removeInlineStyle):
     26        (WebCore::ApplyStyleCommand::addInlineStyleIfNeeded):
     27        * editing/ApplyStyleCommand.h:
     28        * editing/CompositeEditCommand.cpp:
     29        (WebCore::CompositeEditCommand::replaceNodeWithSpanPreservingChildrenAndAttributes):
     30        * editing/CompositeEditCommand.h:
     31        * editing/RemoveNodePreservingChildrenCommand.cpp:
     32        (WebCore::RemoveNodePreservingChildrenCommand::RemoveNodePreservingChildrenCommand):
     33        * editing/ReplaceNodeWithSpanCommand.cpp: Added.
     34        (WebCore::ReplaceNodeWithSpanCommand::ReplaceNodeWithSpanCommand):
     35        (WebCore::swapInNodePreservingAttributesAndChildren):
     36        (WebCore::ReplaceNodeWithSpanCommand::doApply):
     37        (WebCore::ReplaceNodeWithSpanCommand::doUnapply):
     38        * editing/ReplaceNodeWithSpanCommand.h: Added.
     39        (WebCore::ReplaceNodeWithSpanCommand::create):
     40
    1412009-06-04  Brent Fulgham  <bfulgham@webkit.org>
    242

  • trunk/WebCore/WebCore.xcodeproj/project.pbxproj

    r44395 r44435  
    25642564                A89943280B42338800D7C802 /* BitmapImage.h in Headers */ = {isa = PBXBuildFile; fileRef = A89943260B42338700D7C802 /* BitmapImage.h */; };
    25652565                A89943290B42338800D7C802 /* BitmapImage.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A89943270B42338700D7C802 /* BitmapImage.cpp */; };
     2566                A89CCC520F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A89CCC500F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.cpp */; };
     2567                A89CCC530F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.h in Headers */ = {isa = PBXBuildFile; fileRef = A89CCC510F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.h */; };
    25662568                A8A909AC0CBCD6B50029B807 /* RenderSVGTransformableContainer.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A909AA0CBCD6B50029B807 /* RenderSVGTransformableContainer.h */; };
    25672569                A8A909AD0CBCD6B50029B807 /* RenderSVGTransformableContainer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A909AB0CBCD6B50029B807 /* RenderSVGTransformableContainer.cpp */; };
    … …  
    73007302                A89943260B42338700D7C802 /* BitmapImage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BitmapImage.h; sourceTree = "<group>"; };
    73017303                A89943270B42338700D7C802 /* BitmapImage.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = BitmapImage.cpp; sourceTree = "<group>"; };
     7304                A89CCC500F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ReplaceNodeWithSpanCommand.cpp; sourceTree = "<group>"; };
     7305                A89CCC510F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ReplaceNodeWithSpanCommand.h; sourceTree = "<group>"; };
    73027306                A8A909AA0CBCD6B50029B807 /* RenderSVGTransformableContainer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RenderSVGTransformableContainer.h; sourceTree = "<group>"; };
    73037307                A8A909AB0CBCD6B50029B807 /* RenderSVGTransformableContainer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RenderSVGTransformableContainer.cpp; sourceTree = "<group>"; };
    … …  
    1145611460                                93309DB7099E64910056E581 /* RemoveNodePreservingChildrenCommand.cpp */,
    1145711461                                93309DB8099E64910056E581 /* RemoveNodePreservingChildrenCommand.h */,
     11462                                A89CCC500F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.cpp */,
     11463                                A89CCC510F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.h */,
    1145811464                                93309DBA099E64910056E581 /* ReplaceSelectionCommand.cpp */,
    1145911465                                93309DBB099E64910056E581 /* ReplaceSelectionCommand.h */,
    … …  
    1678216788                                845E72FC0FD2623900A87D79 /* SVGFilter.h in Headers */,
    1678316789                                081EBF3B0FD34F4100DA7559 /* SVGFilterBuilder.h in Headers */,
     16790                                A89CCC530F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.h in Headers */,
    1678416791                        );
    1678516792                        runOnlyForDeploymentPostprocessing = 0;
    … …  
    1877418781                                845E72FB0FD2623900A87D79 /* SVGFilter.cpp in Sources */,
    1877518782                                081EBF3A0FD34F4100DA7559 /* SVGFilterBuilder.cpp in Sources */,
     18783                                A89CCC520F44E98100B5DA10 /* ReplaceNodeWithSpanCommand.cpp in Sources */,
    1877618784                        );
    1877718785                        runOnlyForDeploymentPostprocessing = 0;

  • trunk/WebCore/css/CSSStyleDeclaration.h

    r34627 r44435  
    4343
    4444    virtual unsigned length() const = 0;
     45    bool isEmpty() const { return !length(); }
    4546    virtual String item(unsigned index) const = 0;
    4647

  • trunk/WebCore/dom/NamedAttrMap.h

    r42107 r44435  
    6464    PassRefPtr<Node> item(unsigned index) const;
    6565    size_t length() const { return m_attributes.size(); }
     66    bool isEmpty() const { return !length(); }
    6667
    6768    // Internal interface.

  • trunk/WebCore/editing/ApplyStyleCommand.cpp

    r44367 r44435  
    262262    const HTMLElement* elem = static_cast<const HTMLElement*>(node);
    263263    CSSMutableStyleDeclaration* inlineStyleDecl = elem->inlineStyleDecl();
    264     return (!inlineStyleDecl || inlineStyleDecl->length() == 0) && elem->getAttribute(classAttr) == styleSpanClassString();
     264    return (!inlineStyleDecl || inlineStyleDecl->isEmpty()) && elem->getAttribute(classAttr) == styleSpanClassString();
    265265}
    266266
    … …  
    272272    const HTMLElement* elem = static_cast<const HTMLElement*>(node);
    273273    NamedNodeMap* attributes = elem->attributes(true); // readonly
    274     if (attributes->length() == 0)
     274    if (attributes->isEmpty())
    275275        return true;
    276276
    … …  
    432432    while (paragraphStart.isNotNull() && paragraphStart != beyondEnd) {
    433433        StyleChange styleChange(style, paragraphStart.deepEquivalent());
    434         if (styleChange.cssStyle().length() > 0 || m_removeOnly) {
     434        if (styleChange.cssStyle().length() || m_removeOnly) {
    435435            RefPtr<Node> block = enclosingBlock(paragraphStart.deepEquivalent().node());
    436436            RefPtr<Node> newBlock = moveParagraphContentsToNewBlockIfNecessary(paragraphStart.deepEquivalent());
    … …  
    570570            setNodeAttribute(element.get(), styleAttr, inlineStyleDecl->cssText());
    571571        }
    572         if (inlineStyleDecl->length() == 0) {
     572        if (inlineStyleDecl->isEmpty()) {
    573573            removeNodeAttribute(element.get(), styleAttr);
    574574            // FIXME: should this be isSpanWithoutAttributesOrUnstyleStyleSpan?  Need a test.
    … …  
    941941// This function maps from styling tags to CSS styles.  Used for knowing which
    942942// styling tags should be removed when toggling styles.
    943 bool ApplyStyleCommand::isHTMLStyleNode(CSSMutableStyleDeclaration* style, HTMLElement* elem)
     943bool ApplyStyleCommand::implicitlyStyledElementShouldBeRemovedWhenApplyingStyle(HTMLElement* elem, CSSMutableStyleDeclaration* style)
    944944{
    945945    CSSMutableStyleDeclaration::const_iterator end = style->end();
    946946    for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
    947         switch ((*it).id()) {
     947        const CSSProperty& property = *it;
     948        // FIXME: This should probably be re-written to lookup the tagname in a
     949        // hash and match against an expected property/value pair.
     950        switch (property.id()) {
    948951        case CSSPropertyFontWeight:
    949952            // IE inserts "strong" tags for execCommand("bold"), so we remove them, even though they're not strictly presentational
    … …  
    959962            if (elem->hasLocalName(iTag) || elem->hasLocalName(emTag))
    960963                return true;
    961         }
    962     }
    963 
     964            break;
     965        }
     966    }
    964967    return false;
    965968}
    966969
    967 void ApplyStyleCommand::removeHTMLStyleNode(HTMLElement *elem)
    968 {
    969     // This node can be removed.
    970     // EDIT FIXME: This does not handle the case where the node
    971     // has attributes. But how often do people add attributes to <B> tags?
    972     // Not so often I think.
    973     ASSERT(elem);
    974     removeNodePreservingChildren(elem);
     970void ApplyStyleCommand::replaceWithSpanOrRemoveIfWithoutAttributes(HTMLElement*& elem)
     971{
     972    bool removeNode = false;
     973
     974    // Similar to isSpanWithoutAttributesOrUnstyleStyleSpan, but does not look for Apple-style-span.
     975    NamedNodeMap* attributes = elem->attributes(true); // readonly
     976    if (!attributes || attributes->isEmpty())
     977        removeNode = true;
     978    else if (attributes->length() == 1 && elem->hasAttribute(styleAttr)) {
     979        // Remove the element even if it has just style='' (this might be redundantly checked later too)
     980        CSSMutableStyleDeclaration* inlineStyleDecl = elem->inlineStyleDecl();
     981        if (!inlineStyleDecl || inlineStyleDecl->isEmpty())
     982            removeNode = true;
     983    }
     984
     985    if (removeNode)
     986        removeNodePreservingChildren(elem);
     987    else {
     988        HTMLElement* newSpanElement = replaceNodeWithSpanPreservingChildrenAndAttributes(elem);
     989        ASSERT(newSpanElement && newSpanElement->inDocument());
     990        elem = newSpanElement;
     991    }
    975992}
    976993
    … …  
    10411058
    10421059    // No need to serialize <foo style=""> if we just removed the last css property
    1043     if (decl->length() == 0)
     1060    if (decl->isEmpty())
    10441061        removeNodeAttribute(elem, styleAttr);
    10451062
    … …  
    11221139    ASSERT(node);
    11231140
    1124     if (!style || !style->cssText().length())
     1141    if (!style || style->cssText().isEmpty())
    11251142        return;
    11261143
    … …  
    11371154       
    11381155    StyleChange styleChange(style, Position(element, 0));
    1139     if (styleChange.cssStyle().length() > 0) {
     1156    if (styleChange.cssStyle().length()) {
    11401157        String cssText = styleChange.cssStyle();
    11411158        CSSMutableStyleDeclaration *decl = element->inlineStyleDecl();
    … …  
    12341251            if (m_styledInlineElement && elem->hasTagName(m_styledInlineElement->tagQName()))
    12351252                removeNodePreservingChildren(elem);
    1236             if (isHTMLStyleNode(style.get(), elem))
    1237                 removeHTMLStyleNode(elem);
    1238             else {
     1253
     1254            if (implicitlyStyledElementShouldBeRemovedWhenApplyingStyle(elem, style.get()))
     1255                replaceWithSpanOrRemoveIfWithoutAttributes(elem);
     1256
     1257            // If the node was converted to a span, the span may still contain relevant
     1258            // styles which must be removed (e.g. <b style='font-weight: bold'>)
     1259            if (elem->inDocument()) {
    12391260                removeHTMLFontStyle(style.get(), elem);
    12401261                removeHTMLBidiEmbeddingStyle(style.get(), elem);
    … …  
    15591580    }
    15601581
    1561     if (styleChange.cssStyle().length() > 0) {
     1582    if (styleChange.cssStyle().length()) {
    15621583        RefPtr<Element> styleElement = createStyleSpanElement(document());
    15631584        styleElement->setAttribute(styleAttr, styleChange.cssStyle());

  • trunk/WebCore/editing/ApplyStyleCommand.h

    r39997 r44435  
    6363
    6464    // style-removal helpers
    65     bool isHTMLStyleNode(CSSMutableStyleDeclaration*, HTMLElement*);
    66     void removeHTMLStyleNode(HTMLElement*);
     65    bool implicitlyStyledElementShouldBeRemovedWhenApplyingStyle(HTMLElement*, CSSMutableStyleDeclaration*);
     66    void replaceWithSpanOrRemoveIfWithoutAttributes(HTMLElement*&);
    6767    void removeHTMLFontStyle(CSSMutableStyleDeclaration*, HTMLElement*);
    6868    void removeHTMLBidiEmbeddingStyle(CSSMutableStyleDeclaration*, HTMLElement*);

  • trunk/WebCore/editing/CompositeEditCommand.cpp

    r44367 r44435  
    5151#include "RemoveNodeCommand.h"
    5252#include "RemoveNodePreservingChildrenCommand.h"
     53#include "ReplaceNodeWithSpanCommand.h"
    5354#include "ReplaceSelectionCommand.h"
    5455#include "RenderBlock.h"
    … …  
    214215    removeNode(node);
    215216    prune(parent.release());
     217}
     218
     219HTMLElement* CompositeEditCommand::replaceNodeWithSpanPreservingChildrenAndAttributes(PassRefPtr<Node> node)
     220{
     221    // It would also be possible to implement all of ReplaceNodeWithSpanCommand
     222    // as a series of existing smaller edit commands.  Someone who wanted to
     223    // reduce the number of edit commands could do so here.
     224    RefPtr<ReplaceNodeWithSpanCommand> command = ReplaceNodeWithSpanCommand::create(node);
     225    applyCommandToComposite(command);
     226    // Returning a raw pointer here is OK because the command is retained by
     227    // applyCommandToComposite (thus retaining the span), and the span is also
     228    // in the DOM tree, and thus alive whie it has a parent.
     229    ASSERT(command->spanElement()->inDocument());
     230    return command->spanElement();
    216231}
    217232

  • trunk/WebCore/editing/CompositeEditCommand.h

    r42507 r44435  
    3434
    3535class CSSStyleDeclaration;
     36class HTMLElement;
    3637class Text;
    3738
    … …  
    7273    void removeChildrenInRange(PassRefPtr<Node>, unsigned from, unsigned to);
    7374    virtual void removeNode(PassRefPtr<Node>);
     75    HTMLElement* replaceNodeWithSpanPreservingChildrenAndAttributes(PassRefPtr<Node>);
    7476    void removeNodePreservingChildren(PassRefPtr<Node>);
    7577    void removeNodeAndPruneAncestors(PassRefPtr<Node>);

  • trunk/WebCore/editing/RemoveNodePreservingChildrenCommand.cpp

    r39456 r44435  
    3333
    3434RemoveNodePreservingChildrenCommand::RemoveNodePreservingChildrenCommand(PassRefPtr<Node> node)
    35     : CompositeEditCommand(node->document()), m_node(node)
     35    : CompositeEditCommand(node->document())
     36    , m_node(node)
    3637{
    3738    ASSERT(m_node);
Note: See TracChangeset for help on using the changeset viewer.