Changeset 68059 in webkit


Ignore:
Timestamp:
Sep 22, 2010 11:30:34 AM (14 years ago)
Author:
rniwa@webkit.org
Message:

2010-09-22 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

queryCommandState returns false for Underline command when no selection is made
​https://bugs.webkit.org/show_bug.cgi?id=17594

The bug was caused by selectionComputedStyle's adding -webkit-text-decorations-in-effect property
to a dummy span used to obtain the computed style when there is a typing style.
Since this property is for internal use only, the CSS parser always stripped the property.
As a result, we were always ignoring the text decorations set by the typing style.

Fixed the bug by making selectionComputedStyle directly merge the computed style of the current
caret position and the typing style. This also eliminates the need for a dummy span element.
Due to the merge, the return value of selectionComputedStyle had to be changed to CSSMutableStyleDeclaration
from CSSComputedStyleDeclaration.

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

  • editing/ApplyStyleCommand.cpp: (WebCore::StyleChange::init): Calls getPropertiesNotIn. (WebCore::getPropertiesNotIn): Renamed from getPropertiesNotInComputedStyle since it takes CSSStyleDeclaration* instead of CSSComputedStyleDeclaration* for the second argument. (WebCore::ApplyStyleCommand::removeNonEditingProperties): Extracted from editingStyleAtPosition. (WebCore::ApplyStyleCommand::editingStyleAtPosition): Calls removeNonEditingProperties.
  • editing/ApplyStyleCommand.h:
  • editing/Editor.cpp: (WebCore::triStateOfStyle): Calls getPropertiesNotIn. Renamed from triStateOfStyleInComputedStyle since it no longer takes CSSComputedStyleDeclaration. (WebCore::Editor::selectionStartHasStyle): Calls selectionComputedStyle and triStateOfStyle. (WebCore::Editor::selectionHasStyle): Ditto. (WebCore::Editor::selectionStartCSSPropertyValue): Calls selectionComputedStyle. (WebCore::Editor::selectionComputedStyle): See above.
  • editing/Editor.h:
  • editing/EditorCommand.cpp: (WebCore::executeToggleStyleInList): Calls selectionComputedStyle.

2010-09-22 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

queryCommandState returns false for Underline command when no selection is made
​https://bugs.webkit.org/show_bug.cgi?id=17594

Added a test to ensure queryCommandValue('underline') and queryCommandValue('strikeThrough')
correctly incorporates the typing style.

  • editing/execCommand/query-text-decoration-with-typing-style-expected.txt: Added.
  • editing/execCommand/query-text-decoration-with-typing-style.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r68034 r68059  
     12010-09-22  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        queryCommandState returns false for Underline command when no selection is made
     6        https://bugs.webkit.org/show_bug.cgi?id=17594
     7
     8        Added a test to ensure queryCommandValue('underline') and queryCommandValue('strikeThrough')
     9        correctly incorporates the typing style.
     10
     11        * editing/execCommand/query-text-decoration-with-typing-style-expected.txt: Added.
     12        * editing/execCommand/query-text-decoration-with-typing-style.html: Added.
     13
    1142010-09-22  Adam Barth  <abarth@webkit.org>
    215

  • trunk/WebCore/ChangeLog

    r68056 r68059  
     12010-09-22  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        queryCommandState returns false for Underline command when no selection is made
     6        https://bugs.webkit.org/show_bug.cgi?id=17594
     7
     8        The bug was caused by selectionComputedStyle's adding -webkit-text-decorations-in-effect property
     9        to a dummy span used to obtain the computed style when there is a typing style.
     10        Since this property is for internal use only, the CSS parser always stripped the property.
     11        As a result, we were always ignoring the text decorations set by the typing style.
     12
     13        Fixed the bug by making selectionComputedStyle directly merge the computed style of the current
     14        caret position and the typing style. This also eliminates the need for a dummy span element.
     15        Due to the merge, the return value of selectionComputedStyle had to be changed to CSSMutableStyleDeclaration
     16        from CSSComputedStyleDeclaration.
     17
     18        Test: editing/execCommand/query-text-decoration-with-typing-style.html
     19
     20        * editing/ApplyStyleCommand.cpp:
     21        (WebCore::StyleChange::init): Calls getPropertiesNotIn.
     22        (WebCore::getPropertiesNotIn): Renamed from getPropertiesNotInComputedStyle since it takes
     23        CSSStyleDeclaration* instead of CSSComputedStyleDeclaration* for the second argument.
     24        (WebCore::ApplyStyleCommand::removeNonEditingProperties): Extracted from editingStyleAtPosition.
     25        (WebCore::ApplyStyleCommand::editingStyleAtPosition): Calls removeNonEditingProperties.
     26        * editing/ApplyStyleCommand.h:
     27        * editing/Editor.cpp:
     28        (WebCore::triStateOfStyle): Calls getPropertiesNotIn. Renamed from triStateOfStyleInComputedStyle
     29        since it no longer takes CSSComputedStyleDeclaration.
     30        (WebCore::Editor::selectionStartHasStyle): Calls selectionComputedStyle and triStateOfStyle.
     31        (WebCore::Editor::selectionHasStyle): Ditto.
     32        (WebCore::Editor::selectionStartCSSPropertyValue): Calls selectionComputedStyle.
     33        (WebCore::Editor::selectionComputedStyle): See above.
     34        * editing/Editor.h:
     35        * editing/EditorCommand.cpp:
     36        (WebCore::executeToggleStyleInList): Calls selectionComputedStyle.
     37
    1382010-09-22  Jamey Hicks  <jamey.hicks@nokia.com>
    239

  • trunk/WebCore/editing/ApplyStyleCommand.cpp

    r68014 r68059  
    146146
    147147    RefPtr<CSSComputedStyleDeclaration> computedStyle = position.computedStyle();
    148     RefPtr<CSSMutableStyleDeclaration> mutableStyle = getPropertiesNotInComputedStyle(style.get(), computedStyle.get());
     148    RefPtr<CSSMutableStyleDeclaration> mutableStyle = getPropertiesNotIn(style.get(), computedStyle.get());
    149149
    150150    reconcileTextDecorationProperties(mutableStyle.get());
    … …  
    157157
    158158    // If unicode-bidi is present in mutableStyle and direction is not, then add direction to mutableStyle.
    159     // FIXME: Shouldn't this be done in getPropertiesNotInComputedStyle?
     159    // FIXME: Shouldn't this be done in getPropertiesNotIn?
    160160    if (mutableStyle->getPropertyCSSValue(CSSPropertyUnicodeBidi) && !style->getPropertyCSSValue(CSSPropertyDirection))
    161161        mutableStyle->setProperty(CSSPropertyDirection, style->getPropertyValue(CSSPropertyDirection));
    … …  
    389389}
    390390
    391 RefPtr<CSSMutableStyleDeclaration> getPropertiesNotInComputedStyle(CSSStyleDeclaration* style, CSSComputedStyleDeclaration* computedStyle)
    392 {
    393     ASSERT(style);
     391RefPtr<CSSMutableStyleDeclaration> getPropertiesNotIn(CSSStyleDeclaration* styleWithRedundantProperties, CSSStyleDeclaration* baseStyle)
     392{
     393    ASSERT(styleWithRedundantProperties);
    394394    ASSERT(computedStyle);
    395     RefPtr<CSSMutableStyleDeclaration> result = style->copy();
    396     computedStyle->diff(result.get());
    397 
    398     RefPtr<CSSValue> computedTextDecorationsInEffect = computedStyle->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
    399     diffTextDecorations(result.get(), CSSPropertyTextDecoration, computedTextDecorationsInEffect.get());
    400     diffTextDecorations(result.get(), CSSPropertyWebkitTextDecorationsInEffect, computedTextDecorationsInEffect.get());
    401 
    402     if (fontWeightIsBold(result.get()) == fontWeightIsBold(computedStyle))
     395    RefPtr<CSSMutableStyleDeclaration> result = styleWithRedundantProperties->copy();
     396    baseStyle->diff(result.get());
     397
     398    RefPtr<CSSValue> baseTextDecorationsInEffect = baseStyle->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
     399    diffTextDecorations(result.get(), CSSPropertyTextDecoration, baseTextDecorationsInEffect.get());
     400    diffTextDecorations(result.get(), CSSPropertyWebkitTextDecorationsInEffect, baseTextDecorationsInEffect.get());
     401
     402    if (fontWeightIsBold(result.get()) == fontWeightIsBold(baseStyle))
    403403        result->removeProperty(CSSPropertyFontWeight);
    404404
    405     if (getRGBAFontColor(result.get()) == getRGBAFontColor(computedStyle))
     405    if (getRGBAFontColor(result.get()) == getRGBAFontColor(baseStyle))
    406406        result->removeProperty(CSSPropertyColor);
    407407
    … …  
    440440size_t numEditingStyleProperties = sizeof(editingStyleProperties)/sizeof(editingStyleProperties[0]);
    441441
     442RefPtr<CSSMutableStyleDeclaration> ApplyStyleCommand::removeNonEditingProperties(CSSStyleDeclaration* style)
     443{
     444    return style->copyPropertiesInSet(editingStyleProperties, numEditingStyleProperties);
     445}
     446
    442447PassRefPtr<CSSMutableStyleDeclaration> ApplyStyleCommand::editingStyleAtPosition(Position pos, ShouldIncludeTypingStyle shouldIncludeTypingStyle)
    443448{
    … …  
    447452        style = CSSMutableStyleDeclaration::create();
    448453    else
    449         style = computedStyleAtPosition->copyPropertiesInSet(editingStyleProperties, numEditingStyleProperties);
     454        style = removeNonEditingProperties(computedStyleAtPosition.get());
    450455
    451456    if (style && pos.node() && pos.node()->computedStyle()) {

  • trunk/WebCore/editing/ApplyStyleCommand.h

    r68014 r68059  
    5858        return adoptRef(new ApplyStyleCommand(element, removeOnly, action));
    5959    }
    60    
     60
     61    static RefPtr<CSSMutableStyleDeclaration> removeNonEditingProperties(CSSStyleDeclaration* style);
    6162    static PassRefPtr<CSSMutableStyleDeclaration> editingStyleAtPosition(Position pos, ShouldIncludeTypingStyle shouldIncludeTypingStyle = IgnoreTypingStyle);
    6263
    … …  
    126127bool isStyleSpan(const Node*);
    127128PassRefPtr<HTMLElement> createStyleSpanElement(Document*);
    128 RefPtr<CSSMutableStyleDeclaration> getPropertiesNotInComputedStyle(CSSStyleDeclaration* style, CSSComputedStyleDeclaration* computedStyle);
     129RefPtr<CSSMutableStyleDeclaration> getPropertiesNotIn(CSSStyleDeclaration* styleWithRedundantProperties, CSSStyleDeclaration* baseStyle);
    129130
    130131void prepareEditingStyleToApplyAt(CSSMutableStyleDeclaration*, Position);

  • trunk/WebCore/editing/Editor.cpp

    r67653 r68059  
    863863};
    864864
    865 static TriState triStateOfStyleInComputedStyle(CSSStyleDeclaration* desiredStyle, CSSComputedStyleDeclaration* computedStyle, bool ignoreTextOnlyProperties = false)
    866 {
    867     RefPtr<CSSMutableStyleDeclaration> diff = getPropertiesNotInComputedStyle(desiredStyle, computedStyle);
     865static TriState triStateOfStyle(CSSStyleDeclaration* desiredStyle, CSSStyleDeclaration* styleToCompare, bool ignoreTextOnlyProperties = false)
     866{
     867    RefPtr<CSSMutableStyleDeclaration> diff = getPropertiesNotIn(desiredStyle, styleToCompare);
    868868
    869869    if (ignoreTextOnlyProperties)
    … …  
    879879bool Editor::selectionStartHasStyle(CSSStyleDeclaration* style) const
    880880{
    881     Node* nodeToRemove;
    882     RefPtr<CSSComputedStyleDeclaration> selectionStyle = selectionComputedStyle(nodeToRemove);
     881    bool shouldUseFixedFontDefaultSize;
     882    RefPtr<CSSMutableStyleDeclaration> selectionStyle = selectionComputedStyle(shouldUseFixedFontDefaultSize);
    883883    if (!selectionStyle)
    884884        return false;
    885     TriState state = triStateOfStyleInComputedStyle(style, selectionStyle.get());
    886     if (nodeToRemove) {
    887         ExceptionCode ec = 0;
    888         nodeToRemove->remove(ec);
    889         ASSERT(!ec);
    890     }
    891     return state == TrueTriState;
     885    return triStateOfStyle(style, selectionStyle.get()) == TrueTriState;
    892886}
    893887
    … …  
    897891
    898892    if (!m_frame->selection()->isRange()) {
    899         Node* nodeToRemove;
    900         RefPtr<CSSComputedStyleDeclaration> selectionStyle = selectionComputedStyle(nodeToRemove);
     893        bool shouldUseFixedFontDefaultSize;
     894        RefPtr<CSSMutableStyleDeclaration> selectionStyle = selectionComputedStyle(shouldUseFixedFontDefaultSize);
    901895        if (!selectionStyle)
    902896            return FalseTriState;
    903         state = triStateOfStyleInComputedStyle(style, selectionStyle.get());
    904         if (nodeToRemove) {
    905             ExceptionCode ec = 0;
    906             nodeToRemove->remove(ec);
    907             ASSERT(!ec);
    908         }
     897        state = triStateOfStyle(style, selectionStyle.get());
    909898    } else {
    910899        for (Node* node = m_frame->selection()->start().node(); node; node = node->traverseNextNode()) {
    911900            RefPtr<CSSComputedStyleDeclaration> nodeStyle = computedStyle(node);
    912901            if (nodeStyle) {
    913                 TriState nodeState = triStateOfStyleInComputedStyle(style, nodeStyle.get(), !node->isTextNode());
     902                TriState nodeState = triStateOfStyle(style, nodeStyle.get(), !node->isTextNode());
    914903                if (node == m_frame->selection()->start().node())
    915904                    state = nodeState;
    … …  
    945934String Editor::selectionStartCSSPropertyValue(int propertyID)
    946935{
    947     Node* nodeToRemove;
    948     RefPtr<CSSComputedStyleDeclaration> selectionStyle = selectionComputedStyle(nodeToRemove);
     936    bool shouldUseFixedFontDefaultSize = false;
     937    RefPtr<CSSMutableStyleDeclaration> selectionStyle = selectionComputedStyle(shouldUseFixedFontDefaultSize);
    949938    if (!selectionStyle)
    950939        return String();
    … …  
    959948        ExceptionCode ec = 0;
    960949        for (Node* ancestor = range->commonAncestorContainer(ec); ancestor; ancestor = ancestor->parentNode()) {
    961             selectionStyle = computedStyle(ancestor);
     950            selectionStyle = computedStyle(ancestor)->copy();
    962951            if (!hasTransparentBackgroundColor(selectionStyle.get())) {
    963952                value = selectionStyle->getPropertyValue(CSSPropertyBackgroundColor);
    … …  
    971960        ASSERT(cssValue->isPrimitiveValue());
    972961        int fontPixelSize = static_cast<CSSPrimitiveValue*>(cssValue.get())->getIntValue(CSSPrimitiveValue::CSS_PX);
    973         int size = CSSStyleSelector::legacyFontSize(m_frame->document(), fontPixelSize, selectionStyle->useFixedFontDefaultSize());
     962        int size = CSSStyleSelector::legacyFontSize(m_frame->document(), fontPixelSize, shouldUseFixedFontDefaultSize);
    974963        value = String::number(size);
    975     }
    976 
    977     if (nodeToRemove) {
    978         ExceptionCode ec = 0;
    979         nodeToRemove->remove(ec);
    980         ASSERT(!ec);
    981964    }
    982965
    … …  
    32473230}
    32483231
    3249 PassRefPtr<CSSComputedStyleDeclaration> Editor::selectionComputedStyle(Node*& nodeToRemove) const
    3250 {
    3251     nodeToRemove = 0;
    3252 
     3232PassRefPtr<CSSMutableStyleDeclaration> Editor::selectionComputedStyle(bool& shouldUseFixedFontDefaultSize) const
     3233{
    32533234    if (m_frame->selection()->isNone())
    32543235        return 0;
    … …  
    32703251
    32713252    RefPtr<Element> styleElement = element;
    3272     ExceptionCode ec = 0;
    3273 
    3274     if (m_frame->selection()->typingStyle()) {
    3275         styleElement = m_frame->document()->createElement(spanTag, false);
    3276 
    3277         styleElement->setAttribute(styleAttr, m_frame->selection()->typingStyle()->cssText(), ec);
    3278         ASSERT(!ec);
    3279 
    3280         styleElement->appendChild(m_frame->document()->createEditingTextNode(""), ec);
    3281         ASSERT(!ec);
    3282 
    3283         if (element->renderer() && element->renderer()->canHaveChildren())
    3284             element->appendChild(styleElement, ec);
    3285         else {
    3286             Node* parent = element->parent();
    3287             Node* next = element->nextSibling();
    3288 
    3289             if (next)
    3290                 parent->insertBefore(styleElement, next, ec);
    3291             else
    3292                 parent->appendChild(styleElement, ec);
    3293         }
    3294         ASSERT(!ec);
    3295 
    3296         nodeToRemove = styleElement.get();
    3297     }
    3298 
    3299     return computedStyle(styleElement.release());
     3253    RefPtr<CSSComputedStyleDeclaration> style = computedStyle(styleElement.release());
     3254    RefPtr<CSSMutableStyleDeclaration> mutableStyle = style->copy();
     3255    shouldUseFixedFontDefaultSize = style->useFixedFontDefaultSize();
     3256
     3257    if (!m_frame->selection()->typingStyle())
     3258        return mutableStyle;
     3259
     3260    RefPtr<CSSMutableStyleDeclaration> typingStyle = m_frame->selection()->typingStyle()->copy();
     3261    ApplyStyleCommand::removeNonEditingProperties(typingStyle.get());
     3262    prepareEditingStyleToApplyAt(typingStyle.get(), position);
     3263    mutableStyle->merge(typingStyle.get());
     3264
     3265    return mutableStyle;
    33003266}
    33013267

  • trunk/WebCore/editing/Editor.h

    r67534 r68059  
    348348    void setMarkedTextMatchesAreHighlighted(bool);
    349349
    350     PassRefPtr<CSSComputedStyleDeclaration> selectionComputedStyle(Node*& nodeToRemove) const;
     350    PassRefPtr<CSSMutableStyleDeclaration> selectionComputedStyle(bool& shouldUseFixedFontDefaultSize) const;
    351351
    352352    void textFieldDidBeginEditing(Element*);

  • trunk/WebCore/editing/EditorCommand.cpp

    r67534 r68059  
    132132{
    133133    ExceptionCode ec = 0;
    134     Node* nodeToRemove = 0;
    135     RefPtr<CSSComputedStyleDeclaration> selectionStyle = frame->editor()->selectionComputedStyle(nodeToRemove);
     134    bool shouldUseFixedFontDefaultSize;
     135    RefPtr<CSSMutableStyleDeclaration> selectionStyle = frame->editor()->selectionComputedStyle(shouldUseFixedFontDefaultSize);
    136136    if (!selectionStyle)
    137137        return false;
    … …  
    148148    } else if (selectedCSSValue->cssText() == "none")
    149149        newStyle = value->cssText();
    150 
    151     ASSERT(!ec);
    152     if (nodeToRemove) {
    153         nodeToRemove->remove(ec);
    154         ASSERT(!ec);
    155     }
    156150
    157151    // FIXME: We shouldn't be having to convert new style into text.  We should have setPropertyCSSValue.
Note: See TracChangeset for help on using the changeset viewer.