Changeset 55762 in webkit


Ignore:
Timestamp:
Mar 9, 2010 9:32:21 PM (14 years ago)
Author:
tony@chromium.org
Message:

2010-03-09 Tony Chang <​tony@chromium.org>

Reviewed by Adam Barth.

​https://bugs.webkit.org/show_bug.cgi?id=21840
​https://bugs.webkit.org/show_bug.cgi?id=23993

Fix an editing bug where replacing a selection would result in the
new text ending up inside nodes that were not visibly included in the
selection. Instead, move our destination position out of nodes that
were not visibly included.

Two new tests to verify the new behavior. Because we're now inserting
outside of some formatting nodes, some span tags are no longer necessary
for undoing formatting caused by these formatting nodes.

  • editing/deleting/backspace-avoid-preceding-style-expected.txt: Added.
  • editing/deleting/backspace-avoid-preceding-style.html: Added.
  • editing/inserting/replace-at-visible-boundary-expected.txt: Added.
  • editing/inserting/replace-at-visible-boundary.html: Added.
  • platform/mac/editing/deleting/delete-3857753-fix-expected.txt:
  • platform/mac/editing/inserting/insert-div-026-expected.txt:
  • platform/mac/editing/pasteboard/paste-text-at-tabspan-001-expected.txt:
  • platform/mac/editing/pasteboard/paste-text-at-tabspan-002-expected.txt:
  • platform/mac/editing/style/font-family-with-space-expected.txt:
  • platform/mac/editing/style/smoosh-styles-001-expected.txt:
  • platform/mac/editing/style/style-boundary-005-expected.txt:

2010-03-09 Tony Chang <​tony@chromium.org>

Reviewed by Adam Barth.

​https://bugs.webkit.org/show_bug.cgi?id=21840
​https://bugs.webkit.org/show_bug.cgi?id=23993

Fix an editing bug where replacing a selection would result in the
new text ending up inside nodes that were not visibly included in the
selection. Instead, move our destination position out of nodes that
were not visibly included.

Tests: editing/deleting/backspace-avoid-preceding-style.html

editing/inserting/replace-at-visible-boundary.html

  • editing/ReplaceSelectionCommand.cpp: (WebCore::positionAvoidingPrecedingNodes): (WebCore::ReplaceSelectionCommand::doApply):
Location:
trunk
Files:
4 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r55756 r55762  
     12010-03-09  Tony Chang  <tony@chromium.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=21840
     6        https://bugs.webkit.org/show_bug.cgi?id=23993
     7
     8        Fix an editing bug where replacing a selection would result in the
     9        new text ending up inside nodes that were not visibly included in the
     10        selection.  Instead, move our destination position out of nodes that
     11        were not visibly included.
     12
     13        Two new tests to verify the new behavior.  Because we're now inserting
     14        outside of some formatting nodes, some span tags are no longer necessary
     15        for undoing formatting caused by these formatting nodes.
     16
     17        * editing/deleting/backspace-avoid-preceding-style-expected.txt: Added.
     18        * editing/deleting/backspace-avoid-preceding-style.html: Added.
     19        * editing/inserting/replace-at-visible-boundary-expected.txt: Added.
     20        * editing/inserting/replace-at-visible-boundary.html: Added.
     21        * platform/mac/editing/deleting/delete-3857753-fix-expected.txt:
     22        * platform/mac/editing/inserting/insert-div-026-expected.txt:
     23        * platform/mac/editing/pasteboard/paste-text-at-tabspan-001-expected.txt:
     24        * platform/mac/editing/pasteboard/paste-text-at-tabspan-002-expected.txt:
     25        * platform/mac/editing/style/font-family-with-space-expected.txt:
     26        * platform/mac/editing/style/smoosh-styles-001-expected.txt:
     27        * platform/mac/editing/style/style-boundary-005-expected.txt:
     28
    1292010-03-09  Csaba Osztrogonác  <ossy@webkit.org>
    230

  • trunk/LayoutTests/platform/mac/editing/deleting/delete-3857753-fix-expected.txt

    r46446 r55762  
    1919    RenderBody {BODY} at (8,8) size 784x584 [border: (2px solid #FF0000)]
    2020      RenderBlock {DIV} at (14,14) size 756x28
    21         RenderInline {B} at (0,0) size 37x28
     21        RenderInline {B} at (0,0) size 25x28
    2222          RenderText {#text} at (0,0) size 25x28
    2323            text run at (0,0) width 25: "on"
    24           RenderInline {SPAN} at (0,0) size 12x28
    25             RenderInline {I} at (0,0) size 12x28
    26               RenderText {#text} at (25,0) size 12x28
    27                 text run at (25,0) width 12: "o"
     24        RenderInline {I} at (0,0) size 12x28
     25          RenderText {#text} at (25,0) size 12x28
     26            text run at (25,0) width 12: "o"
    2827      RenderBlock {DIV} at (14,42) size 756x28
    2928        RenderInline {B} at (0,0) size 54x28

  • trunk/LayoutTests/platform/mac/editing/inserting/insert-div-026-expected.txt

    r25970 r55762  
    5353      RenderBlock {DIV} at (0,236) size 784x32
    5454        RenderBlock {DIV} at (0,0) size 784x32 [border: (2px solid #FF0000)]
    55           RenderInline {B} at (0,0) size 32x28
     55          RenderInline {B} at (0,0) size 20x28
    5656            RenderText {#text} at (2,2) size 20x28
    5757              text run at (2,2) width 20: "fo"
    58             RenderInline {SPAN} at (0,0) size 12x28
    59               RenderText {#text} at (22,2) size 12x28
    60                 text run at (22,2) width 12: "x"
     58          RenderText {#text} at (22,2) size 12x28
     59            text run at (22,2) width 12: "x"
    6160caret: position 3 of child 0 {#text} of child 0 {B} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document

  • trunk/LayoutTests/platform/mac/editing/pasteboard/paste-text-at-tabspan-001-expected.txt

    r25970 r55762  
    55EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    66EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
    7 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     7EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    88EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    99EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
    10 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     10EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1111EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1212EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
    … …  
    2020          RenderText {#text} at (14,14) size 11x28
    2121            text run at (14,14) width 11: "a"
    22           RenderText {#text} at (25,14) size 23x28
    23             text run at (25,14) width 23: "ax"
    24           RenderInline {SPAN} at (0,0) size 110x28
     22          RenderInline {SPAN} at (0,0) size 133x28
     23            RenderInline {SPAN} at (0,0) size 23x28
     24              RenderText {#text} at (25,14) size 23x28
     25                text run at (25,14) width 23: "ax"
    2526            RenderText {#text} at (48,14) size 110x28
    2627              text run at (48,14) width 110: "\x{9}\x{9}\x{9}"
    … …  
    2829            text run at (158,14) width 11: "z"
    2930        RenderText {#text} at (0,0) size 0x0
    30 caret: position 2 of child 1 {#text} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
     31caret: position 2 of child 0 {#text} of child 0 {SPAN} of child 1 {SPAN} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document

  • trunk/LayoutTests/platform/mac/editing/pasteboard/paste-text-at-tabspan-002-expected.txt

    r25970 r55762  
    66EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    77EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
    8 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     8EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    99EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1010EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
    11 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     11EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 1 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > SPAN > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1212EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    1313EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
    … …  
    2424            RenderText {#text} at (25,14) size 37x28
    2525              text run at (25,14) width 37: "\x{9}"
    26           RenderText {#text} at (62,14) size 23x28
    27             text run at (62,14) width 23: "ax"
    28           RenderInline {SPAN} at (0,0) size 73x28
     26          RenderInline {SPAN} at (0,0) size 96x28
     27            RenderInline {SPAN} at (0,0) size 23x28
     28              RenderText {#text} at (62,14) size 23x28
     29                text run at (62,14) width 23: "ax"
    2930            RenderText {#text} at (85,14) size 73x28
    3031              text run at (85,14) width 73: "\x{9}\x{9}"
    … …  
    3233            text run at (158,14) width 11: "z"
    3334        RenderText {#text} at (0,0) size 0x0
    34 caret: position 2 of child 2 {#text} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
     35caret: position 2 of child 0 {#text} of child 0 {SPAN} of child 2 {SPAN} of child 1 {SPAN} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document

  • trunk/LayoutTests/platform/mac/editing/style/font-family-with-space-expected.txt

    r47640 r55762  
    44  RenderBlock {HTML} at (0,0) size 800x600
    55    RenderBody {BODY} at (8,8) size 784x584
    6       RenderInline {U} at (0,0) size 490x15
     6      RenderInline {U} at (0,0) size 245x15
    77        RenderText {#text} at (0,0) size 245x15
    88          text run at (0,0) width 245: "This text should be Times New Roman bold."
    9         RenderInline {SPAN} at (0,0) size 245x15
    10           RenderInline {U} at (0,0) size 245x15
    11             RenderText {#text} at (245,0) size 245x15
    12               text run at (245,0) width 245: "This text should be Times New Roman bold."
     9      RenderInline {U} at (0,0) size 245x15
     10        RenderText {#text} at (245,0) size 245x15
     11          text run at (245,0) width 245: "This text should be Times New Roman bold."
    1312      RenderText {#text} at (0,0) size 0x0
    1413      RenderText {#text} at (0,0) size 0x0
    15 caret: position 41 of child 0 {#text} of child 0 {U} of child 1 {SPAN} of child 0 {U} of child 1 {BODY} of child 0 {HTML} of document
     14caret: position 41 of child 0 {#text} of child 1 {U} of child 1 {BODY} of child 0 {HTML} of document

  • trunk/LayoutTests/platform/mac/editing/style/smoosh-styles-001-expected.txt

    r25970 r55762  
    1919EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
    2020EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    21 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > SPAN > SPAN > DIV > DIV > BODY > HTML > #document to 3 of #text > SPAN > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     21EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > DIV > DIV > BODY > HTML > #document to 3 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    2222EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    2323EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
    … …  
    5252      RenderBlock {DIV} at (0,208) size 784x32
    5353        RenderBlock {DIV} at (0,0) size 784x32 [border: (2px solid #FF0000)]
    54           RenderInline {SPAN} at (0,0) size 77x28 [color=#FF0000]
     54          RenderInline {SPAN} at (0,0) size 23x28 [color=#FF0000]
    5555            RenderText {#text} at (2,2) size 23x28
    5656              text run at (2,2) width 23: "ab"
    57             RenderInline {SPAN} at (0,0) size 54x28 [color=#000000]
    58               RenderText {#text} at (25,2) size 34x28
    59                 text run at (25,2) width 34: "cde"
    60               RenderInline {SPAN} at (0,0) size 20x28 [color=#FF0000]
    61                 RenderText {#text} at (59,2) size 20x28
    62                   text run at (59,2) width 20: "fg"
     57          RenderText {#text} at (25,2) size 34x28
     58            text run at (25,2) width 34: "cde"
     59          RenderInline {SPAN} at (0,0) size 20x28 [color=#FF0000]
     60            RenderText {#text} at (59,2) size 20x28
     61              text run at (59,2) width 20: "fg"
    6362        RenderBlock (anonymous) at (0,32) size 784x0
    64 caret: position 3 of child 0 {#text} of child 1 {SPAN} of child 1 {SPAN} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
     63caret: position 3 of child 2 {#text} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document

  • trunk/LayoutTests/platform/mac/editing/style/style-boundary-005-expected.txt

    r34799 r55762  
    2525EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    2626EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 5 of #text > B > DIV > DIV > BODY > HTML > #document to 5 of #text > B > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
    27 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > B > DIV > DIV > BODY > HTML > #document to 5 of #text > B > DIV > DIV > BODY > HTML > #document toDOMRange:range from 4 of #text > SPAN > B > DIV > DIV > BODY > HTML > #document to 4 of #text > SPAN > B > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     27EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > B > DIV > DIV > BODY > HTML > #document to 5 of #text > B > DIV > DIV > BODY > HTML > #document toDOMRange:range from 4 of #text > DIV > DIV > BODY > HTML > #document to 4 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    2828EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    2929EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
    … …  
    6060          RenderText {#text} at (2,2) size 86x18
    6161            text run at (2,2) width 86: "one two three"
    62           RenderInline {B} at (0,0) size 60x18
     62          RenderInline {B} at (0,0) size 33x18
    6363            RenderText {#text} at (88,2) size 33x18
    6464              text run at (88,2) width 33: " four"
    65             RenderInline {SPAN} at (0,0) size 27x18
    66               RenderText {#text} at (121,2) size 27x18
    67                 text run at (121,2) width 27: " one"
     65          RenderText {#text} at (121,2) size 27x18
     66            text run at (121,2) width 27: " one"
    6867        RenderBlock (anonymous) at (0,22) size 784x0
    6968          RenderText {#text} at (0,0) size 0x0
    70 caret: position 4 of child 0 {#text} of child 1 {SPAN} of child 1 {B} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
     69caret: position 4 of child 2 {#text} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document

  • trunk/WebCore/ChangeLog

    r55761 r55762  
     12010-03-09  Tony Chang  <tony@chromium.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=21840
     6        https://bugs.webkit.org/show_bug.cgi?id=23993
     7
     8        Fix an editing bug where replacing a selection would result in the
     9        new text ending up inside nodes that were not visibly included in the
     10        selection.  Instead, move our destination position out of nodes that
     11        were not visibly included.
     12
     13        Tests: editing/deleting/backspace-avoid-preceding-style.html
     14               editing/inserting/replace-at-visible-boundary.html
     15
     16        * editing/ReplaceSelectionCommand.cpp:
     17        (WebCore::positionAvoidingPrecedingNodes):
     18        (WebCore::ReplaceSelectionCommand::doApply):
     19
    1202010-03-09  Brady Eidson  <beidson@apple.com>
    221

  • trunk/WebCore/editing/ReplaceSelectionCommand.cpp

    r55178 r55762  
    105105}
    106106
     107static Position positionAvoidingPrecedingNodes(Position pos)
     108{
     109    // If we're already on a break, it's probably a placeholder and we shouldn't change our position.
     110    if (pos.node()->hasTagName(brTag))
     111        return pos;
     112
     113    // We also stop when changing block flow elements because even though the visual position is the
     114    // same.  E.g.,
     115    //   <div>foo^</div>^
     116    // The two positions above are the same visual position, but we want to stay in the same block.
     117    Node* stopNode = pos.node()->enclosingBlockFlowElement();
     118    while (stopNode != pos.node() && VisiblePosition(pos) == VisiblePosition(pos.next()))
     119        pos = pos.next();
     120    return pos;
     121}
     122
    107123ReplacementFragment::ReplacementFragment(Document* document, DocumentFragment* fragment, bool matchStyle, const VisibleSelection& selection)
    108124    : m_document(document),
    … …  
    886902   
    887903    bool handledStyleSpans = handleStyleSpansBeforeInsertion(fragment, insertionPos);
    888    
     904
     905    // We don't want the destination to end up inside nodes that weren't selected.  To avoid that, we move the
     906    // position forward without changing the visible position so we're still at the same visible location, but
     907    // outside of preceding tags.
     908    insertionPos = positionAvoidingPrecedingNodes(insertionPos);
     909
    889910    // FIXME: When pasting rich content we're often prevented from heading down the fast path by style spans.  Try
    890911    // again here if they've been removed.
Note: See TracChangeset for help on using the changeset viewer.