Changeset 86852 in webkit


Ignore:
Timestamp:
May 19, 2011 10:22:24 AM (13 years ago)
Author:
rniwa@webkit.org
Message:

2011-05-19 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Darin Adler.

REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
https://bugs.webkit.org/show_bug.cgi?id=61012

Added a test to ensure WebKit does not crash when inserting a content immediately after
a styled element inside a Mail blockquote. Regrettably the expected result is incorrect,
but it matches the behavior of WebKit before r83322.

  • editing/pasteboard/5065605-expected.txt: Reintroduced redundant style spans.
  • editing/pasteboard/paste-text-011-expected.txt: Ditto.
  • platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
  • editing/pasteboard/paste-after-inline-style-element-expected.txt: Added.
  • editing/pasteboard/paste-after-inline-style-element.html: Added.

2011-05-19 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Darin Adler.

REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
https://bugs.webkit.org/show_bug.cgi?id=61012

The crash was caused by ReplaceSelectionCommand's inserting content into a middle of the paragraph
being moved when the insertion position's container node is the node to split to. Fixed the crash
by not changing the insertion position in such a case.

Unfortunately, this fix caused markup to bloat in some tests but we'll take this regression since
it's much better than crashing.

Test: editing/pasteboard/paste-after-inline-style-element.html

  • editing/ReplaceSelectionCommand.cpp: (WebCore::ReplaceSelectionCommand::doApply):
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r86846 r86852  
     12011-05-19  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
     6        https://bugs.webkit.org/show_bug.cgi?id=61012
     7
     8        Added a test to ensure WebKit does not crash when inserting a content immediately after
     9        a styled element inside a Mail blockquote. Regrettably the expected result is incorrect,
     10        but it matches the behavior of WebKit before r83322.
     11
     12        * editing/pasteboard/5065605-expected.txt: Reintroduced redundant style spans.
     13        * editing/pasteboard/paste-text-011-expected.txt: Ditto.
     14        * platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
     15        * editing/pasteboard/paste-after-inline-style-element-expected.txt: Added.
     16        * editing/pasteboard/paste-after-inline-style-element.html: Added.
     17
    1182011-05-19  Csaba Osztrogonác  <ossy@webkit.org>
    219
  • trunk/LayoutTests/editing/pasteboard/5065605-expected.txt

    r83268 r86852  
    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 > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     24EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    2525EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    2626EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
     
    4141|     class="Apple-style-span"
    4242|     color="#ff0000"
    43 |     "This text should be red."
    44 |   <div>
    45 |     <font>
     43|     <span>
    4644|       class="Apple-style-span"
    47 |       color="#ff0000"
    48 |       "This text should be red.<#selection-caret>"
     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|       style="color: rgb(0, 0, 0); "
     52|       <font>
     53|         class="Apple-style-span"
     54|         color="#ff0000"
     55|         "This text should be red.<#selection-caret>"
  • trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt

    r83246 r86852  
    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 > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     9EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1010EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1111EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
     
    3030|   <font>
    3131|     face="Monaco"
    32 |     <p>
    33 |       style="font-family: Times; "
    34 |       <font>
    35 |         face="Monaco"
    36 |         <b>
    37 |           "hello"
    38 |     <p>
    39 |       style="font-family: Times; "
    40 |       <font>
    41 |         face="Monaco"
    42 |         <b>
    43 |           "there<#selection-caret>"
     32|     <b>
     33|       <p>
     34|         style="font-family: Times; font-weight: normal; "
     35|         <font>
     36|           face="Monaco"
     37|           <b>
     38|             "hello"
     39|       <p>
     40|         style="font-family: Times; font-weight: normal; "
     41|         <font>
     42|           face="Monaco"
     43|           <b>
     44|             "there<#selection-caret>"
    4445| "
    4546
  • trunk/LayoutTests/platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt

    r83262 r86852  
    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 > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     9EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1010EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1111EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
     
    3030|   <font>
    3131|     face="Monaco"
    32 |     <p>
    33 |       style="font-family: 'times new roman'; "
    34 |       <font>
    35 |         face="Monaco"
    36 |         <b>
    37 |           "hello"
    38 |     <p>
    39 |       style="font-family: 'times new roman'; "
    40 |       <font>
    41 |         face="Monaco"
    42 |         <b>
    43 |           "there<#selection-caret>"
     32|     <b>
     33|       <p>
     34|         style="font-family: 'times new roman'; font-weight: normal; "
     35|         <font>
     36|           face="Monaco"
     37|           <b>
     38|             "hello"
     39|       <p>
     40|         style="font-family: 'times new roman'; font-weight: normal; "
     41|         <font>
     42|           face="Monaco"
     43|           <b>
     44|             "there<#selection-caret>"
    4445| "
    4546
  • trunk/Source/WebCore/ChangeLog

    r86847 r86852  
     12011-05-19  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
     6        https://bugs.webkit.org/show_bug.cgi?id=61012
     7
     8        The crash was caused by ReplaceSelectionCommand's inserting content into a middle of the paragraph
     9        being moved when the insertion position's container node is the node to split to. Fixed the crash
     10        by not changing the insertion position in such a case.
     11
     12        Unfortunately, this fix caused markup to bloat in some tests but we'll take this regression since
     13        it's much better than crashing.
     14
     15        Test: editing/pasteboard/paste-after-inline-style-element.html
     16
     17        * editing/ReplaceSelectionCommand.cpp:
     18        (WebCore::ReplaceSelectionCommand::doApply):
     19
    1202011-05-19  Brady Eidson  <beidson@apple.com>
    221
  • trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp

    r86360 r86852  
    981981        // FIXME: isInlineNodeWithStyle does not check editability.
    982982        if (RefPtr<Node> nodeToSplitTo = highestEnclosingNodeOfType(insertionPos, isInlineNodeWithStyle)) {
    983             if (insertionPos.containerNode() != nodeToSplitTo)
     983            if (insertionPos.containerNode() != nodeToSplitTo) {
    984984                nodeToSplitTo = splitTreeToNode(insertionPos.anchorNode(), nodeToSplitTo.get(), true).get();
    985             insertionPos = positionInParentBeforeNode(nodeToSplitTo.get());
     985                insertionPos = positionInParentBeforeNode(nodeToSplitTo.get());
     986            }
    986987        }
    987988    }
Note: See TracChangeset for help on using the changeset viewer.