Changeset 92330 in webkit


Ignore:
Timestamp:
Aug 3, 2011 4:42:25 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

select-all, copy, paste of specialAncestorElements (e.g. pre, h1, etc) nests the element inside itself
https://bugs.webkit.org/show_bug.cgi?id=26483

Reviewed by Enrica Casucci.

Source/WebCore:

The bug was caused by WebKit serializing pre, h1, etc... to retain structure and appearance when copying
rich content and pasting did not remove such nodes wrapping the copied contents.

Fixed the bug by extending r81887 and r83322 to remove those elements from where WebKit pastes into.

Test: editing/pasteboard/copy-paste-text-in-h1.html

  • editing/ReplaceSelectionCommand.cpp:

(WebCore::nodeHasAttributesToPreserve): Extracted from isInlineNodeWithStyle.
(WebCore::isInlineNodeWithStyle): Calls nodeHasAttributesToPreserve.
(WebCore::ReplaceSelectionCommand::doApply): Calls ancestorToRetainStructureAndAppearance.
Remove nodes copied by ancestorToRetainStructureAndAppearance at insertionPos before pasting the fragment.

  • editing/markup.cpp:

(WebCore::ancestorToRetainStructureAndAppearance): Takes ShouldIncludeParagraphSeparators.

  • editing/markup.h:

LayoutTests:

  • editing/pasteboard/5065605-expected.txt:
  • editing/pasteboard/copy-paste-text-in-h1-expected.txt: Added.
  • editing/pasteboard/copy-paste-text-in-h1.html: Added.
  • editing/pasteboard/display-block-on-spans-expected.txt:
  • editing/pasteboard/paste-pre-001-expected.txt:
  • editing/pasteboard/paste-pre-002-expected.txt:
  • editing/pasteboard/paste-text-011-expected.txt:
Location:
trunk
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r92327 r92330  
     12011-08-03  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        select-all, copy, paste of specialAncestorElements (e.g. pre, h1, etc) nests the element inside itself
     4        https://bugs.webkit.org/show_bug.cgi?id=26483
     5
     6        Reviewed by Enrica Casucci.
     7
     8        * editing/pasteboard/5065605-expected.txt:
     9        * editing/pasteboard/copy-paste-text-in-h1-expected.txt: Added.
     10        * editing/pasteboard/copy-paste-text-in-h1.html: Added.
     11        * editing/pasteboard/display-block-on-spans-expected.txt:
     12        * editing/pasteboard/paste-pre-001-expected.txt:
     13        * editing/pasteboard/paste-pre-002-expected.txt:
     14        * editing/pasteboard/paste-text-011-expected.txt:
     15
    1162011-08-03  Mark Pilgrim  <pilgrim@chromium.org>
    217
  • trunk/LayoutTests/editing/pasteboard/5065605-expected.txt

    r86983 r92330  
    2222EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
    2323EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    24 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     24EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    2525EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    2626EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
     
    3737|     color="#ff0000"
    3838|     "This text should be red."
     39| <font>
     40|   class="Apple-style-span"
     41|   color="#ff0000"
     42|   "This text should be red."
    3943| <div>
    4044|   <font>
    4145|     class="Apple-style-span"
    4246|     color="#ff0000"
    43 |     <span>
    44 |       class="Apple-style-span"
    45 |       style="color: rgb(0, 0, 0); "
    46 |       <font>
    47 |         class="Apple-style-span"
    48 |         color="#ff0000"
    49 |         "This text should be red."
    50 |       <div>
    51 |         <font>
    52 |           class="Apple-style-span"
    53 |           color="#ff0000"
    54 |           "This text should be red.<#selection-caret>"
     47|     "This text should be red.<#selection-caret>"
  • trunk/LayoutTests/editing/pasteboard/display-block-on-spans-expected.txt

    r83268 r92330  
    3030|   style="display:block"
    3131|   <b>
    32 |     "This<#selection-caret>"
    33 |   <b>
     32|     <span>
     33|       class="Apple-style-span"
     34|       style="font-weight: normal; "
     35|       <b>
     36|         "This<#selection-caret>"
    3437|     " is another paragraph."
    3538|   <br>
  • trunk/LayoutTests/editing/pasteboard/paste-pre-001-expected.txt

    r87531 r92330  
    88bar
    99execCutCommand: <div id="test" class="editing"> <pre><br></pre> </div>
    10 execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: Times; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
     10execPasteCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
  • trunk/LayoutTests/editing/pasteboard/paste-pre-002-expected.txt

    r87531 r92330  
    33bar
    44execCopyCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
    5 execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: Times; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
     5execPasteCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
  • trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt

    r86983 r92330  
    77EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
    88EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    9 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     9EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1010EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1111EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
     
    3131|     face="Monaco"
    3232|     <b>
    33 |       <span>
    34 |         class="Apple-style-span"
    35 |         style="font-family: Times; font-weight: normal; "
    36 |         <p>
    37 |           <font>
    38 |             face="Monaco"
    39 |             <b>
    40 |               "hello"
    41 |         <p>
    42 |           <font>
    43 |             face="Monaco"
    44 |             <b>
    45 |               "there<#selection-caret>"
     33|       "hello"
     34| <p>
     35|   <font>
     36|     face="Monaco"
     37|     <b>
     38|       "there<#selection-caret>"
    4639| "
    4740
  • trunk/Source/WebCore/ChangeLog

    r92328 r92330  
     12011-08-03  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        select-all, copy, paste of specialAncestorElements (e.g. pre, h1, etc) nests the element inside itself
     4        https://bugs.webkit.org/show_bug.cgi?id=26483
     5
     6        Reviewed by Enrica Casucci.
     7
     8        The bug was caused by WebKit serializing pre, h1, etc... to retain structure and appearance when copying
     9        rich content and pasting did not remove such nodes wrapping the copied contents.
     10
     11        Fixed the bug by extending r81887 and r83322 to remove those elements from where WebKit pastes into.
     12
     13        Test: editing/pasteboard/copy-paste-text-in-h1.html
     14
     15        * editing/ReplaceSelectionCommand.cpp:
     16        (WebCore::nodeHasAttributesToPreserve): Extracted from isInlineNodeWithStyle.
     17        (WebCore::isInlineNodeWithStyle): Calls nodeHasAttributesToPreserve.
     18        (WebCore::ReplaceSelectionCommand::doApply): Calls ancestorToRetainStructureAndAppearance.
     19        Remove nodes copied by ancestorToRetainStructureAndAppearance at insertionPos before pasting the fragment.
     20        * editing/markup.cpp:
     21        (WebCore::ancestorToRetainStructureAndAppearance): Takes ShouldIncludeParagraphSeparators.
     22        * editing/markup.h:
     23
    1242011-08-03  Mark Pilgrim  <pilgrim@chromium.org>
    225
  • trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp

    r91702 r92330  
    761761}
    762762
     763static bool nodeHasAttributesToPreserve(const HTMLElement* element)
     764{
     765    const NamedNodeMap* attributeMap = element->attributeMap();
     766    return attributeMap && !attributeMap->isEmpty() && (attributeMap->length() > 1 || !element->hasAttribute(styleAttr));
     767}
     768
    763769static bool isInlineNodeWithStyle(const Node* node)
    764770{
     
    782788    // We can skip inline elements that don't have attributes or whose only
    783789    // attribute is the style attribute.
    784     const NamedNodeMap* attributeMap = element->attributeMap();
    785     if (!attributeMap || attributeMap->isEmpty() || (attributeMap->length() == 1 && element->hasAttribute(styleAttr)))
    786         return true;
    787 
    788     return false;
     790    return !nodeHasAttributesToPreserve(static_cast<const HTMLElement*>(node));
    789791}
    790792   
     
    938940    // our style spans and for positions inside list items
    939941    // since insertAsListItems already does the right thing.
    940     if (!m_matchStyle && !enclosingList(insertionPos.containerNode()) && isStyleSpan(fragment.firstChild())) {
     942    if (!m_matchStyle && !enclosingList(insertionPos.containerNode()) && isStyleSpan(fragment.firstChild())
     943        && VisiblePosition(firstPositionInNode(insertionPos.containerNode())) == VisiblePosition(lastPositionInNode(insertionPos.containerNode()))) {
    941944        if (insertionPos.containerNode()->isTextNode() && insertionPos.offsetInContainerNode() && !insertionPos.atLastEditingPositionForNode()) {
    942945            splitTextNodeContainingElement(insertionPos.containerText(), insertionPos.offsetInContainerNode());
     
    944947        }
    945948
    946         // FIXME: isInlineNodeWithStyle does not check editability.
    947         if (RefPtr<Node> nodeToSplitTo = highestEnclosingNodeOfType(insertionPos, isInlineNodeWithStyle)) {
    948             if (insertionPos.containerNode() != nodeToSplitTo) {
    949                 nodeToSplitTo = splitTreeToNode(insertionPos.anchorNode(), nodeToSplitTo.get(), true).get();
     949        RefPtr<Node> nodeToSplitTo = highestEnclosingNodeOfType(insertionPos, isInlineNodeWithStyle);
     950        if (HTMLElement* ancestor = ancestorToRetainStructureAndAppearance(nodeToSplitTo ? nodeToSplitTo.get() : insertionPos.containerNode(), IncludeParagraphSeparators)) {
     951            if (ancestor->parentNode() && unsplittableElementForPosition(insertionPos)->contains(ancestor->parentNode()) && !nodeHasAttributesToPreserve(ancestor))
     952                nodeToSplitTo = ancestor;
     953        }
     954        if (nodeToSplitTo) {
     955            if (insertionPos.containerNode() != nodeToSplitTo->parentNode()) {
     956                nodeToSplitTo = splitTreeToNode(insertionPos.anchorNode(), nodeToSplitTo->parentNode()).get();
    950957                insertionPos = positionInParentBeforeNode(nodeToSplitTo.get());
    951958            }
  • trunk/Source/WebCore/editing/markup.cpp

    r89440 r92330  
    351351}
    352352
    353 static Node* ancestorToRetainStructureAndAppearance(Node* commonAncestor)
     353HTMLElement* ancestorToRetainStructureAndAppearance(Node* commonAncestor, ShouldIncludeParagraphSeparators shouldIncludeParagraphSeparators)
    354354{
    355355    Node* commonAncestorBlock = enclosingBlock(commonAncestor);
     
    363363            table = table->parentNode();
    364364
    365         return table;
     365        return toHTMLElement(table);
    366366    }
    367367
     
    377377        || commonAncestorBlock->hasTagName(h4Tag)
    378378        || commonAncestorBlock->hasTagName(h5Tag))
    379         return commonAncestorBlock;
     379        return toHTMLElement(commonAncestorBlock);
     380
     381    if (shouldIncludeParagraphSeparators == IncludeParagraphSeparators
     382        && (commonAncestorBlock->hasTagName(pTag) || commonAncestorBlock->hasTagName(divTag)))
     383        return toHTMLElement(commonAncestorBlock);
    380384
    381385    return 0;
  • trunk/Source/WebCore/editing/markup.h

    r85064 r92330  
    3434namespace WebCore {
    3535
    36     class Document;
    37     class DocumentFragment;
    38     class Element;
    39     class KURL;
    40     class Node;
    41     class Range;
     36class Document;
     37class DocumentFragment;
     38class Element;
     39class HTMLElement;
     40class KURL;
     41class Node;
     42class Range;
    4243
    43     enum EChildrenOnly { IncludeNode, ChildrenOnly };
    44     enum EAbsoluteURLs { DoNotResolveURLs, AbsoluteURLs };
     44enum EChildrenOnly { IncludeNode, ChildrenOnly };
     45enum EAbsoluteURLs { DoNotResolveURLs, AbsoluteURLs };
    4546
    46     PassRefPtr<DocumentFragment> createFragmentFromText(Range* context, const String& text);
    47     PassRefPtr<DocumentFragment> createFragmentFromMarkup(Document*, const String& markup, const String& baseURL, FragmentScriptingPermission = FragmentScriptingAllowed);
    48     PassRefPtr<DocumentFragment> createFragmentFromNodes(Document*, const Vector<Node*>&);
     47PassRefPtr<DocumentFragment> createFragmentFromText(Range* context, const String& text);
     48PassRefPtr<DocumentFragment> createFragmentFromMarkup(Document*, const String& markup, const String& baseURL, FragmentScriptingPermission = FragmentScriptingAllowed);
     49PassRefPtr<DocumentFragment> createFragmentFromNodes(Document*, const Vector<Node*>&);
    4950
    50     bool isPlainTextMarkup(Node *node);
     51bool isPlainTextMarkup(Node*);
    5152
    52     String createMarkup(const Range*,
    53         Vector<Node*>* = 0, EAnnotateForInterchange = DoNotAnnotateForInterchange, bool convertBlocksToInlines = false, EAbsoluteURLs = DoNotResolveURLs);
    54     String createMarkup(const Node*, EChildrenOnly = IncludeNode, Vector<Node*>* = 0, EAbsoluteURLs = DoNotResolveURLs);
    55    
    56     String createFullMarkup(const Node*);
    57     String createFullMarkup(const Range*);
     53String createMarkup(const Range*, Vector<Node*>* = 0, EAnnotateForInterchange = DoNotAnnotateForInterchange, bool convertBlocksToInlines = false, EAbsoluteURLs = DoNotResolveURLs);
     54String createMarkup(const Node*, EChildrenOnly = IncludeNode, Vector<Node*>* = 0, EAbsoluteURLs = DoNotResolveURLs);
    5855
    59     String urlToMarkup(const KURL&, const String& title);
    60     String imageToMarkup(const KURL&, Element*);
     56String createFullMarkup(const Node*);
     57String createFullMarkup(const Range*);
     58
     59String urlToMarkup(const KURL&, const String& title);
     60String imageToMarkup(const KURL&, Element*);
     61
     62enum ShouldIncludeParagraphSeparators { DoNotIncludeParagraphSeparators, IncludeParagraphSeparators };
     63HTMLElement* ancestorToRetainStructureAndAppearance(Node*, ShouldIncludeParagraphSeparators = DoNotIncludeParagraphSeparators);
     64
    6165}
    6266
Note: See TracChangeset for help on using the changeset viewer.