Changeset 92620 in webkit


Ignore:
Timestamp:
Aug 8, 2011 12:15:16 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

Remove redundant inline styles from the pasted contents more aggressively
https://bugs.webkit.org/show_bug.cgi?id=65833

Reviewed by Tony Chang.

Source/WebCore:

Make removeRedundantStylesAndKeepStyleSpanInline more aggressively remove redundant styles by realizing that
when an editing inheritable property in an inline style declaration of an element can be safely removed
if it is present either in style rules or in its ancestor and not overridden by style rules or default style
of the element.

Test: editing/pasteboard/paste-with-redundant-style.html

  • css/CSSStyleSelector.cpp:

(WebCore::CSSStyleSelector::pseudoStyleRulesForElement): Do not match author style sheets if AuthorCSSRules
is not included in the rules to include. This is used in EditingStyle::removePropertiesInElementDefaultStyle.

  • editing/ApplyStyleCommand.cpp:

(WebCore::isStyleSpanOrSpanWithOnlyStyleAttribute): Added; returns true if the element is a style span or
span possibly with a style attribute.

  • editing/ApplyStyleCommand.h:
  • editing/EditingStyle.cpp:

(WebCore::styleFromMatchedRulesForElement): Takes rulesToInclude.
(WebCore::EditingStyle::mergeStyleFromRules): Calls styleFromMatchedRulesForElement with AuthorCSSRules
| CrossOriginCSSRules to keep the original behavior.
(WebCore::EditingStyle::removeStyleFromRulesAndContext): Renamed from removeStyleFromRules; removes styles that
are present in context and not overridden by matched rules.
(WebCore::EditingStyle::removePropertiesInElementDefaultStyle): Added.

  • editing/EditingStyle.h:
  • editing/ReplaceSelectionCommand.cpp:

(WebCore::ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline): See the description above.

LayoutTests:

Added a test to paste contents with many redundant styles. WebKit should remove as much style spans as possible
(all of this in this case).

  • editing/pasteboard/4930986-2-expected.txt: style attribute now has a trailing space.
  • editing/pasteboard/interchange-newline-1-expected.txt: No longer has a wrapping span without any attributes.
  • editing/pasteboard/nested-blocks-with-text-area-expected.txt: Ditto.
  • editing/pasteboard/nested-blocks-with-text-field-expected.txt: Ditto.
  • editing/pasteboard/paste-blockquote-into-blockquote-2-expected.txt: Ditto.
  • editing/pasteboard/paste-blockquote-into-blockquote-expected.txt: Ditto.
  • editing/pasteboard/prevent-block-nesting-01-expected.txt: Ditto.
  • editing/pasteboard/paste-with-redundant-style-expected.txt: Added.
  • editing/pasteboard/paste-with-redundant-style.html: Added.
Location:
trunk
Files:
2 added
15 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r92618 r92620  
     12011-08-08  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Remove redundant inline styles from the pasted contents more aggressively
     4        https://bugs.webkit.org/show_bug.cgi?id=65833
     5
     6        Reviewed by Tony Chang.
     7
     8        Added a test to paste contents with many redundant styles. WebKit should remove as much style spans as possible
     9        (all of this in this case).
     10
     11        * editing/pasteboard/4930986-2-expected.txt: style attribute now has a trailing space.
     12        * editing/pasteboard/interchange-newline-1-expected.txt: No longer has a wrapping span without any attributes.
     13        * editing/pasteboard/nested-blocks-with-text-area-expected.txt: Ditto.
     14        * editing/pasteboard/nested-blocks-with-text-field-expected.txt: Ditto.
     15        * editing/pasteboard/paste-blockquote-into-blockquote-2-expected.txt: Ditto.
     16        * editing/pasteboard/paste-blockquote-into-blockquote-expected.txt: Ditto.
     17        * editing/pasteboard/prevent-block-nesting-01-expected.txt: Ditto.
     18        * editing/pasteboard/paste-with-redundant-style-expected.txt: Added.
     19        * editing/pasteboard/paste-with-redundant-style.html: Added.
     20
    1212011-08-08  Oliver Hunt  <oliver@apple.com>
    222
  • trunk/LayoutTests/editing/pasteboard/4930986-2-expected.txt

    r86983 r92620  
    11This tests to make sure that content that is colored by the user is pasted with that color during a Paste as Quotation.
    2 <blockquote><span class="Apple-style-span" style="color: red;">This text should be red (it should be wrapped in a style span).</span></blockquote>
     2<blockquote><span class="Apple-style-span" style="color: red; ">This text should be red (it should be wrapped in a style span).</span></blockquote>
  • trunk/LayoutTests/editing/pasteboard/interchange-newline-1-expected.txt

    r87476 r92620  
    1111The paragraph "bar" is inside a div wrapped in a span, and the old paste code that handled interchange newlines did not handle this case.
    1212| "x"
    13 | <span>
    14 |   "foo"
    15 |   <div>
    16 |     "bar"
     13| "foo"
     14| <div>
     15|   "bar"
    1716| "<#selection-caret>x"
  • trunk/LayoutTests/editing/pasteboard/nested-blocks-with-text-area-expected.txt

    r92578 r92620  
    66EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    77EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    8 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     8EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > DIV > DIV > BODY > HTML > #document to 1 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    99EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1010EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
     
    3131|     "
    3232"
    33 |     <span>
    34 |       "x<#selection-caret>"
     33|     "x<#selection-caret>"
    3534|     "
    3635"
  • trunk/LayoutTests/editing/pasteboard/nested-blocks-with-text-field-expected.txt

    r92578 r92620  
    66EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    77EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    8 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     8EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > DIV > DIV > BODY > HTML > #document to 1 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    99EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1010EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
     
    3030|     "
    3131"
    32 |     <span>
    33 |       "x<#selection-caret>"
     32|     "x<#selection-caret>"
    3433|     "
    3534"
  • trunk/LayoutTests/editing/pasteboard/paste-blockquote-into-blockquote-2-expected.txt

    r87775 r92620  
    55|   "One"
    66|   "Two"
    7 | <span>
     7| <blockquote>
     8|   type="cite"
    89|   <blockquote>
    910|     type="cite"
    10 |     <blockquote>
    11 |       type="cite"
    12 |       <div>
    13 |         "Three<#selection-caret>"
     11|     <div>
     12|       "Three<#selection-caret>"
  • trunk/LayoutTests/editing/pasteboard/paste-blockquote-into-blockquote-expected.txt

    r92580 r92620  
    55|   "One"
    66|   "Two"
    7 | <span>
    8 |   <blockquote>
    9 |     type="cite"
    10 |     <div>
    11 |       "Three<#selection-caret>"
     7| <blockquote>
     8|   type="cite"
     9|   <div>
     10|     "Three<#selection-caret>"
  • trunk/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.txt

    r92580 r92620  
    1010The code in paste that prevents block nesting had a bug where the order of pasted paragraphs could be reversed.
    1111| "There should be an empty line between these two paragraphs."
    12 | <span>
    13 |   <div>
    14 |     <br>
     12| <div>
     13|   <br>
    1514| <div>
    1615|   "This paragraph and the empty line should have be in their own divs with a red border.<#selection-caret>"
  • trunk/Source/WebCore/ChangeLog

    r92619 r92620  
     12011-08-08  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Remove redundant inline styles from the pasted contents more aggressively
     4        https://bugs.webkit.org/show_bug.cgi?id=65833
     5
     6        Reviewed by Tony Chang.
     7
     8        Make removeRedundantStylesAndKeepStyleSpanInline more aggressively remove redundant styles by realizing that
     9        when an editing inheritable property in an inline style declaration of an element can be safely removed
     10        if it is present either in style rules or in its ancestor and not overridden by style rules or default style
     11        of the element.
     12
     13        Test: editing/pasteboard/paste-with-redundant-style.html
     14
     15        * css/CSSStyleSelector.cpp:
     16        (WebCore::CSSStyleSelector::pseudoStyleRulesForElement): Do not match author style sheets if AuthorCSSRules
     17        is not included in the rules to include. This is used in EditingStyle::removePropertiesInElementDefaultStyle.
     18        * editing/ApplyStyleCommand.cpp:
     19        (WebCore::isStyleSpanOrSpanWithOnlyStyleAttribute): Added; returns true if the element is a style span or
     20        span possibly with a style attribute.
     21        * editing/ApplyStyleCommand.h:
     22        * editing/EditingStyle.cpp:
     23        (WebCore::styleFromMatchedRulesForElement): Takes rulesToInclude.
     24        (WebCore::EditingStyle::mergeStyleFromRules): Calls styleFromMatchedRulesForElement with AuthorCSSRules
     25        | CrossOriginCSSRules to keep the original behavior.
     26        (WebCore::EditingStyle::removeStyleFromRulesAndContext): Renamed from removeStyleFromRules; removes styles that
     27        are present in context and not overridden by matched rules.
     28        (WebCore::EditingStyle::removePropertiesInElementDefaultStyle): Added.
     29        * editing/EditingStyle.h:
     30        * editing/ReplaceSelectionCommand.cpp:
     31        (WebCore::ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline): See the description above.
     32
    1332011-08-08  Dmitry Lomov  <dslomov@google.com>
    234
  • trunk/Source/WebCore/css/CSSStyleSelector.cpp

    r92457 r92620  
    20592059    }
    20602060
    2061     if (m_matchAuthorAndUserStyles) {
     2061    if (m_matchAuthorAndUserStyles && (rulesToInclude & AuthorCSSRules)) {
    20622062        m_checker.m_sameOriginOnly = !(rulesToInclude & CrossOriginCSSRules);
    20632063
  • trunk/Source/WebCore/editing/ApplyStyleCommand.cpp

    r89952 r92620  
    7171}
    7272
     73bool isStyleSpanOrSpanWithOnlyStyleAttribute(const Element* element)
     74{
     75    if (!element || !element->hasTagName(spanTag))
     76        return false;
     77
     78    const bool readonly = true;
     79    NamedNodeMap* map = element->attributes(readonly);
     80    if (!map || map->isEmpty())
     81        return true;
     82
     83    unsigned matchedAttributes = 0;
     84    if (element->fastGetAttribute(classAttr) == styleSpanClassString())
     85        matchedAttributes++;
     86    if (element->hasAttribute(styleAttr))
     87        matchedAttributes++;
     88
     89    ASSERT(matchedAttributes <= map->length());
     90    return matchedAttributes == map->length();
     91}
     92
    7393static bool isUnstyledStyleSpan(const Node* node)
    7494{
  • trunk/Source/WebCore/editing/ApplyStyleCommand.h

    r83618 r92620  
    131131
    132132bool isStyleSpan(const Node*);
     133bool isStyleSpanOrSpanWithOnlyStyleAttribute(const Element*);
    133134PassRefPtr<HTMLElement> createStyleSpanElement(Document*);
    134135
  • trunk/Source/WebCore/editing/EditingStyle.cpp

    r87952 r92620  
    819819}
    820820
    821 static PassRefPtr<CSSMutableStyleDeclaration> styleFromMatchedRulesForElement(Element* element)
     821static PassRefPtr<CSSMutableStyleDeclaration> styleFromMatchedRulesForElement(Element* element, unsigned rulesToInclude)
    822822{
    823823    RefPtr<CSSMutableStyleDeclaration> style = CSSMutableStyleDeclaration::create();
    824     RefPtr<CSSRuleList> matchedRules = element->document()->styleSelector()->styleRulesForElement(element,
    825         CSSStyleSelector::AuthorCSSRules | CSSStyleSelector::CrossOriginCSSRules);
     824    RefPtr<CSSRuleList> matchedRules = element->document()->styleSelector()->styleRulesForElement(element, rulesToInclude);
    826825    if (matchedRules) {
    827826        for (unsigned i = 0; i < matchedRules->length(); i++) {
     
    838837void EditingStyle::mergeStyleFromRules(StyledElement* element)
    839838{
    840     RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(element);
     839    RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(element,
     840        CSSStyleSelector::AuthorCSSRules | CSSStyleSelector::CrossOriginCSSRules);
    841841    // Styles from the inline style declaration, held in the variable "style", take precedence
    842842    // over those from matched rules.
     
    871871}
    872872
    873 void EditingStyle::removeStyleFromRules(StyledElement* element)
    874 {
     873void EditingStyle::removeStyleFromRulesAndContext(StyledElement* element, Node* context)
     874{
     875    ASSERT(element);
    875876    if (!m_mutableStyle)
    876877        return;
    877878
    878     RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(element);
    879     if (!styleFromMatchedRules)
    880         return;
    881 
    882     m_mutableStyle = getPropertiesNotIn(m_mutableStyle.get(), styleFromMatchedRules.get());
    883 }
     879    // 1. Remove style from matched rules because style remain without repeating it in inline style declaration
     880    RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(element, CSSStyleSelector::AllButEmptyCSSRules);
     881    if (styleFromMatchedRules && styleFromMatchedRules->length())
     882        m_mutableStyle = getPropertiesNotIn(m_mutableStyle.get(), styleFromMatchedRules.get());
     883
     884    // 2. Remove style present in context and not overriden by matched rules.
     885    RefPtr<EditingStyle> computedStyle = EditingStyle::create(context, EditingInheritablePropertiesAndBackgroundColorInEffect);
     886    if (computedStyle->m_mutableStyle) {
     887        computedStyle->removePropertiesInElementDefaultStyle(element);
     888        m_mutableStyle = getPropertiesNotIn(m_mutableStyle.get(), computedStyle->m_mutableStyle.get());
     889    }
     890}
     891
     892void EditingStyle::removePropertiesInElementDefaultStyle(StyledElement* element)
     893{
     894    if (!m_mutableStyle || !m_mutableStyle->length())
     895        return;
     896
     897    RefPtr<CSSMutableStyleDeclaration> defaultStyle = styleFromMatchedRulesForElement(element, CSSStyleSelector::UAAndUserCSSRules);
     898
     899    Vector<int> propertiesToRemove(defaultStyle->length());
     900    size_t i = 0;
     901    CSSMutableStyleDeclaration::const_iterator end = defaultStyle->end();
     902    for (CSSMutableStyleDeclaration::const_iterator it = defaultStyle->begin(); it != end; ++it, ++i)
     903        propertiesToRemove[i] = it->id();
     904
     905    m_mutableStyle->removePropertiesInSet(propertiesToRemove.data(), propertiesToRemove.size());
     906}
     907   
    884908
    885909static void reconcileTextDecorationProperties(CSSMutableStyleDeclaration* style)
  • trunk/Source/WebCore/editing/EditingStyle.h

    r87466 r92620  
    123123    void mergeStyleFromRules(StyledElement*);
    124124    void mergeStyleFromRulesForSerialization(StyledElement*);
    125     void removeStyleFromRules(StyledElement*);
     125    void removeStyleFromRulesAndContext(StyledElement*, Node* context);
    126126
    127127    float fontSizeDelta() const { return m_fontSizeDelta; }
     
    142142    bool conflictsWithInlineStyleOfElement(StyledElement*, EditingStyle* extractedStyle, Vector<CSSPropertyID>* conflictingProperties) const;
    143143    void mergeStyle(CSSMutableStyleDeclaration*);
     144    void removePropertiesInElementDefaultStyle(StyledElement*);
    144145
    145146    RefPtr<CSSMutableStyleDeclaration> m_mutableStyle;
  • trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp

    r92537 r92620  
    479479void ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline()
    480480{
    481     for (RefPtr<Node> node = m_firstNodeInserted.get(); node; node = node->traverseNextNode()) {
     481    RefPtr<Node> pastEndNode = m_lastLeafInserted ? m_lastLeafInserted->traverseNextNode() : 0;
     482    RefPtr<Node> next;
     483    for (RefPtr<Node> node = m_firstNodeInserted.get(); node && node != pastEndNode; node = next) {
    482484        // FIXME: <rdar://problem/5371536> Style rules that match pasted content can change it's appearance
    483         if (isStyleSpan(node.get())) {
    484             HTMLElement* e = toHTMLElement(node.get());
     485
     486        next = node->traverseNextNode();
     487        if (!node->isStyledElement())
     488            continue;
     489
     490        StyledElement* element = static_cast<StyledElement*>(node.get());
     491
     492        CSSMutableStyleDeclaration* inlineStyle = element->inlineStyleDecl();
     493        RefPtr<EditingStyle> newInlineStyle = EditingStyle::create(inlineStyle);
     494        if (inlineStyle) {
     495            ContainerNode* context = element->parentNode();
     496
     497            // If Mail wraps the fragment with a Paste as Quotation blockquote, or if you're pasting into a quoted region,
     498            // styles from blockquoteNode are allowed to override those from the source document, see <rdar://problem/4930986> and <rdar://problem/5089327>.
     499            Node* blockquoteNode = isMailPasteAsQuotationNode(context) ? context : enclosingNodeOfType(firstPositionInNode(context), isMailBlockquote, CanCrossEditingBoundary);
     500            if (blockquoteNode)
     501                newInlineStyle->removeStyleFromRulesAndContext(element, document()->documentElement());
     502
     503            newInlineStyle->removeStyleFromRulesAndContext(element, context);
     504        }
     505
     506        if (!inlineStyle || newInlineStyle->isEmpty()) {
     507            if (isStyleSpanOrSpanWithOnlyStyleAttribute(element))
     508                removeNodePreservingChildren(element);
     509            else
     510                removeNodeAttribute(element, styleAttr);
     511        } else if (newInlineStyle->style()->length() != inlineStyle->length())
     512            setNodeAttribute(element, styleAttr, newInlineStyle->style()->cssText());
     513
     514        if (isStyleSpan(element)) {
     515            if (!element->firstChild()) {
     516                removeNodePreservingChildren(element);
     517                continue;
     518            }
    485519            // There are other styles that style rules can give to style spans,
    486520            // but these are the two important ones because they'll prevent
     
    489523            // results. We already know one issue because td elements ignore their display property
    490524            // in quirks mode (which Mail.app is always in). We should look for an alternative.
    491             if (isBlock(e))
    492                 e->getInlineStyleDecl()->setProperty(CSSPropertyDisplay, CSSValueInline);
    493             if (e->renderer() && e->renderer()->style()->isFloating())
    494                 e->getInlineStyleDecl()->setProperty(CSSPropertyFloat, CSSValueNone);
    495         } else if (node->isStyledElement()) {
    496             StyledElement* element = static_cast<StyledElement*>(node.get());
    497             if (CSSMutableStyleDeclaration* inlineStyle = element->inlineStyleDecl()) {
    498                 RefPtr<EditingStyle> newInlineStyle = EditingStyle::create(inlineStyle);
    499                 newInlineStyle->removeStyleFromRules(element);
    500                 if (!newInlineStyle->style() || !newInlineStyle->style()->length())
    501                     removeNodeAttribute(element, styleAttr);
    502                 else if (newInlineStyle->style()->length() < inlineStyle->length())
    503                     setNodeAttribute(element, styleAttr, newInlineStyle->style()->cssText());                   
    504             }
    505         }
    506 
    507         if (node == m_lastLeafInserted)
    508             break;
     525            if (isBlock(element))
     526                element->getInlineStyleDecl()->setProperty(CSSPropertyDisplay, CSSValueInline);
     527            if (element->renderer() && element->renderer()->style()->isFloating())
     528                element->getInlineStyleDecl()->setProperty(CSSPropertyFloat, CSSValueNone);
     529        }
    509530    }
    510531}
Note: See TracChangeset for help on using the changeset viewer.