Changeset 69354 in webkit


Ignore:
Timestamp:
Oct 7, 2010 4:55:00 PM (14 years ago)
Author:
rniwa@webkit.org
Message:

2010-10-07 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

REGRESSION: Indenting pre duplicates content
​https://bugs.webkit.org/show_bug.cgi?id=47233

The bug was caused by our not splitting text nodes properly.

In new approach, we split text nodes in each iteration. Added rangeForParagraphSplitingTextNodesIfNeeded
to split text nodes at the start and at the end of paragraph, which also adjusts start and end positions
for moveParagraphWithClones. Added endOfNextParagrahSplittingTextNodesIfNeeded to adjust endOfNextParagraph,
start, and end to work-around moveParagraphWithClones's removing a line feed.

Tests: editing/execCommand/indent-pre-list.html

editing/execCommand/indent-pre-paragraphs.html

  • editing/ApplyBlockElementCommand.cpp: (WebCore::ApplyBlockElementCommand::formatSelection): See above. (WebCore::isNewLineAtPosition): (WebCore::renderStyleOfEnclosingTextNode): Added. (WebCore::ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded): Added. (WebCore::ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfNeeded): Added.
  • editing/ApplyBlockElementCommand.h:
  • editing/FormatBlockCommand.cpp: (WebCore::FormatBlockCommand::formatRange): Takes two Positions instead of one VisiblePosition.
  • editing/FormatBlockCommand.h:
  • editing/IndentOutdentCommand.cpp: (WebCore::IndentOutdentCommand::tryIndentingAsListItem): Ditto. (WebCore::IndentOutdentCommand::indentIntoBlockquote): Ditto. (WebCore::IndentOutdentCommand::formatRange): Ditto.
  • editing/IndentOutdentCommand.h:

2010-10-07 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

REGRESSION: Indenting pre duplicates content
​https://bugs.webkit.org/show_bug.cgi?id=47233

Added tests to ensure WebKit indents texts inside pre correctly.

  • editing/execCommand/indent-pre-expected.txt: This test passes if WebKit does not crash.
  • editing/execCommand/indent-pre-list-expected.txt: Added.
  • editing/execCommand/indent-pre-list.html: Added.
  • editing/execCommand/indent-pre-paragraphs-expected.txt: Added.
  • editing/execCommand/indent-pre-paragraphs.html: Added.
Location:
trunk
Files:
4 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r69353 r69354  
     12010-10-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        REGRESSION: Indenting pre duplicates content
     6        https://bugs.webkit.org/show_bug.cgi?id=47233
     7
     8        Added tests to ensure WebKit indents texts inside pre correctly.
     9
     10        * editing/execCommand/indent-pre-expected.txt: This test passes if WebKit does not crash.
     11        * editing/execCommand/indent-pre-list-expected.txt: Added.
     12        * editing/execCommand/indent-pre-list.html: Added.
     13        * editing/execCommand/indent-pre-paragraphs-expected.txt: Added.
     14        * editing/execCommand/indent-pre-paragraphs.html: Added.
     15
    1162010-10-07  Albert J. Wong  <ajwong@chromium.org>
    217

  • trunk/LayoutTests/editing/execCommand/indent-pre-expected.txt

    r64432 r69354  
    11CONSOLE MESSAGE: line 44: Wrong node selected.
    22CONSOLE MESSAGE: line 46: Wrong anchor offset: 8 instead of 0
     3CONSOLE MESSAGE: line 41: Wrong end node type: [object HTMLBRElement]
     4CONSOLE MESSAGE: line 44: Wrong node selected.
    35| <html>
    46|   <head>
    … …  
    1820|         <pre>
    1921|           id="pre-basic"
    20 |           "line one
    21 "
     22|           "line one"
    2223|       <pre>
    2324|         id="pre-basic"
    … …  
    2829|         style="margin: 0 0 0 40px; border: none; padding: 0px;"
    2930|         <pre>
    30 |           "line three
    31 "
     31|           "line three"
    3232|         <pre>
    3333|           "line four"
    … …  
    4444|               class="webkit-indent-blockquote"
    4545|               style="margin: 0 0 0 40px; border: none; padding: 0px;"
    46 |               "list two
    47 "
    48 |               "list three
    49 "
    50 |             "list four
    51 "
     46|               "list two"
     47|               <br>
     48|               "list three"
     49|               <br>
     50|               "list four"
    5251|       "
    5352
    … …  
    6766|                 style="margin: 0 0 0 40px; border: none; padding: 0px;"
    6867|                 <pre>
    69 |                   "table two
    70 "
     68|                   "table two"
    7169|                 <pre>
    7270|                   "table three<#selection-focus>"

  • trunk/WebCore/ChangeLog

    r69349 r69354  
     12010-10-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        REGRESSION: Indenting pre duplicates content
     6        https://bugs.webkit.org/show_bug.cgi?id=47233
     7
     8        The bug was caused by our not splitting text nodes properly.
     9
     10        In new approach, we split text nodes in each iteration. Added rangeForParagraphSplitingTextNodesIfNeeded
     11        to split text nodes at the start and at the end of paragraph, which also adjusts start and end positions
     12        for moveParagraphWithClones. Added endOfNextParagrahSplittingTextNodesIfNeeded to adjust endOfNextParagraph,
     13        start, and end to work-around moveParagraphWithClones's removing a line feed.
     14
     15        Tests: editing/execCommand/indent-pre-list.html
     16               editing/execCommand/indent-pre-paragraphs.html
     17
     18        * editing/ApplyBlockElementCommand.cpp:
     19        (WebCore::ApplyBlockElementCommand::formatSelection): See above.
     20        (WebCore::isNewLineAtPosition):
     21        (WebCore::renderStyleOfEnclosingTextNode): Added.
     22        (WebCore::ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded): Added.
     23        (WebCore::ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfNeeded): Added.
     24        * editing/ApplyBlockElementCommand.h:
     25        * editing/FormatBlockCommand.cpp:
     26        (WebCore::FormatBlockCommand::formatRange): Takes two Positions instead of one VisiblePosition.
     27        * editing/FormatBlockCommand.h:
     28        * editing/IndentOutdentCommand.cpp:
     29        (WebCore::IndentOutdentCommand::tryIndentingAsListItem): Ditto.
     30        (WebCore::IndentOutdentCommand::indentIntoBlockquote): Ditto.
     31        (WebCore::IndentOutdentCommand::formatRange): Ditto.
     32        * editing/IndentOutdentCommand.h:
     33
    1342010-10-07  Jian Li  <jianli@chromium.org>
    235

  • trunk/WebCore/editing/ApplyBlockElementCommand.cpp

    r68972 r69354  
    3939
    4040using namespace HTMLNames;
    41    
    42 // This function can return -1 if we are unable to count the paragraphs between |start| and |end|.
    43 static int countParagraphs(const VisiblePosition& endOfFirstParagraph, const VisiblePosition& endOfLastParagraph)
    44 {
    45     int count = 0;
    46     VisiblePosition cur = endOfFirstParagraph;
    47     while (cur != endOfLastParagraph) {
    48         ++count;
    49         cur = endOfParagraph(cur.next());
    50         // If start is before a table and end is inside a table, we will never hit end because the
    51         // whole table is considered a single paragraph.
    52         if (cur.isNull())
    53             return -1;
    54     }
    55     return count;
    56 }
    5741
    5842ApplyBlockElementCommand::ApplyBlockElementCommand(Document* document, const QualifiedName& tagName, const AtomicString& className, const AtomicString& inlineStyle)
    … …  
    126110    VisiblePosition endOfCurrentParagraph = endOfParagraph(startOfSelection);
    127111    VisiblePosition endAfterSelection = endOfParagraph(endOfParagraph(endOfSelection).next());
    128     int endOfCurrentParagraphIndex = indexForVisiblePosition(endOfCurrentParagraph);
    129     int endAfterSelectionIndex = indexForVisiblePosition(endAfterSelection);
    130 
    131     // When indenting within a <pre> tag, we need to split each paragraph into a separate node for moveParagraphWithClones to work.
    132     // However, splitting text nodes can cause endOfCurrentParagraph and endAfterSelection to point to an invalid position if we
    133     // changed the text node it was pointing at.  So we have to reset these positions.
    134     int numParagraphs = countParagraphs(endOfCurrentParagraph, endAfterSelection);
    135     if (splitTextNodes(startOfParagraph(startOfSelection), numParagraphs + 1)) {
    136         RefPtr<Range> endOfCurrentParagraphRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), endOfCurrentParagraphIndex, 0, true);
    137         RefPtr<Range> endAfterSelectionRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), endAfterSelectionIndex, 0, true);
    138         if (!endOfCurrentParagraphRange.get() || !endAfterSelectionRange.get()) {
    139             ASSERT_NOT_REACHED();
    140             return;
    141         }
    142         endOfCurrentParagraph = VisiblePosition(endOfCurrentParagraphRange->startPosition(), DOWNSTREAM);
    143         endAfterSelection = VisiblePosition(endAfterSelectionRange->startPosition(), DOWNSTREAM);
    144     }
    145 
    146     while (endOfCurrentParagraph != endAfterSelection) {
    147         // Iterate across the selected paragraphs...
    148         VisiblePosition endOfNextParagraph = endOfParagraph(endOfCurrentParagraph.next());
     112    VisiblePosition endOfLastParagraph = endOfParagraph(endOfSelection);
     113
     114    bool atEnd = false;
     115    while (endOfCurrentParagraph != endAfterSelection && !atEnd) {
     116        Position start;
     117        Position end;
     118
     119        if (endOfCurrentParagraph == endOfLastParagraph)
     120            atEnd = true;
     121
     122        rangeForParagraphSplittingTextNodesIfNeeded(endOfCurrentParagraph, start, end);
     123        endOfCurrentParagraph = end;
     124
     125        Position afterEnd = end.next();
    149126        Node* enclosingCell = enclosingNodeOfType(start, &isTableCell);
    150 
    151         formatParagraph(endOfCurrentParagraph, blockquoteForNextIndent);
     127        VisiblePosition endOfNextParagraph = endOfNextParagrahSplittingTextNodesIfNeeded(endOfCurrentParagraph, start, end);
     128
     129        formatRange(start, end, blockquoteForNextIndent);
    152130
    153131        // Don't put the next paragraph in the blockquote we just created for this paragraph unless
    … …  
    171149}
    172150
     151static bool isNewLineAtPosition(const Position& position)
     152{
     153    if (position.anchorType() != Position::PositionIsOffsetInAnchor)
     154        return false;
     155
     156    Node* textNode = position.containerNode();
     157    int offset = position.offsetInContainerNode();
     158    if (!textNode || !textNode->isTextNode() || offset < 0 || offset >= textNode->maxCharacterOffset())
     159        return false;
     160
     161    ExceptionCode ec = 0;
     162    String textAtPosition = static_cast<Text*>(textNode)->substringData(offset, 1, ec);
     163    if (ec)
     164        return false;
     165
     166    return textAtPosition[0] == '\n';
     167}
     168
     169static RenderStyle* renderStyleOfEnclosingTextNode(const Position& position)
     170{
     171    if (position.anchorType() != Position::PositionIsOffsetInAnchor
     172        || !position.containerNode()
     173        || !position.containerNode()->isTextNode()
     174        || !position.containerNode()->renderer())
     175        return 0;
     176    return position.containerNode()->renderer()->style();
     177}
     178
     179void ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded(const VisiblePosition& endOfCurrentParagraph, Position& start, Position& end)
     180{
     181    start = startOfParagraph(endOfCurrentParagraph).deepEquivalent();
     182    end = endOfCurrentParagraph.deepEquivalent();
     183
     184    RenderStyle* startStyle = renderStyleOfEnclosingTextNode(start);
     185    if (startStyle) {
     186        // Avoid obtanining the start of next paragraph for start
     187        if (startStyle->preserveNewline() && isNewLineAtPosition(start) && !isNewLineAtPosition(start.previous()) && start.offsetInContainerNode() > 0)
     188            start = startOfParagraph(end.previous()).deepEquivalent();
     189
     190        // If start is in the middle of a text node, split.
     191        if (!startStyle->collapseWhiteSpace() && start.offsetInContainerNode() > 0) {
     192            int startOffset = start.offsetInContainerNode();
     193            splitTextNode(static_cast<Text*>(start.node()), startOffset);
     194            start = positionBeforeNode(start.node());
     195            if (start.node() == end.node()) {
     196                ASSERT(end.offsetInContainerNode() >= startOffset);
     197                end = Position(end.node(), end.offsetInContainerNode() - startOffset, Position::PositionIsOffsetInAnchor);
     198            }
     199        }
     200    }
     201
     202    RenderStyle* endStyle = renderStyleOfEnclosingTextNode(end);
     203    if (endStyle) {
     204        // Include \n at the end of line if we're at an empty paragraph
     205        if (endStyle->preserveNewline() && start == end
     206            && end.offsetInContainerNode() < end.containerNode()->maxCharacterOffset()) {
     207            int endOffset = end.offsetInContainerNode();
     208            if (!isNewLineAtPosition(end.previous()) && isNewLineAtPosition(end))
     209                end = Position(end.node(), endOffset + 1, Position::PositionIsOffsetInAnchor);
     210        }
     211
     212        // If end is in the middle of a text node, split.
     213        if (!endStyle->collapseWhiteSpace() && end.offsetInContainerNode()
     214            && end.offsetInContainerNode() < end.containerNode()->maxCharacterOffset()) {
     215            splitTextNode(static_cast<Text*>(end.node()), end.offsetInContainerNode());
     216            if (start.node() == end.node())
     217                start = positionBeforeNode(end.node()->previousSibling());
     218            end = lastPositionInNode(end.node()->previousSibling());
     219        }
     220    }
     221}
     222
     223VisiblePosition ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfNeeded(VisiblePosition& endOfCurrentParagraph, Position& start, Position& end)
     224{
     225    VisiblePosition endOfNextParagraph = endOfParagraph(endOfCurrentParagraph.next());
     226    Position position = endOfNextParagraph.deepEquivalent();
     227    RenderStyle* style = renderStyleOfEnclosingTextNode(position);
     228    if (!style)
     229        return endOfNextParagraph;
     230
     231    RefPtr<Node> containerNode = position.containerNode();
     232    if (!style->preserveNewline() || !position.offsetInContainerNode()
     233        || !isNewLineAtPosition(Position(containerNode.get(), 0, Position::PositionIsOffsetInAnchor)))
     234        return endOfNextParagraph;
     235
     236    // \n at the beginning of the text node immediately following the current paragraph is trimmed by moveParagraphWithClones.
     237    // If endOfNextParagraph was pointing at this same text node, endOfNextParagraph will be shifted by one paragraph.
     238    // Avoid this by splitting "\n"
     239    splitTextNode(static_cast<Text*>(containerNode.get()), 1);
     240
     241    if (start.anchorType() == Position::PositionIsOffsetInAnchor && containerNode.get() == start.containerNode()) {
     242        ASSERT(start.offsetInContainerNode() < position.offsetInContainerNode());
     243        start = Position(containerNode->previousSibling(), start.offsetInContainerNode(), Position::PositionIsOffsetInAnchor);
     244    }
     245    if (end.anchorType() == Position::PositionIsOffsetInAnchor && containerNode.get() == end.containerNode()) {
     246        ASSERT(end.offsetInContainerNode() < position.offsetInContainerNode());
     247        end = Position(containerNode->previousSibling(), end.offsetInContainerNode(), Position::PositionIsOffsetInAnchor);
     248    }
     249
     250    return Position(containerNode.get(), position.offsetInContainerNode() - 1, Position::PositionIsOffsetInAnchor);
     251}
     252
    173253PassRefPtr<Element> ApplyBlockElementCommand::createBlockElement() const
    174254{
    … …  
    181261}
    182262
    183 // Returns true if at least one text node was split.
    184 bool ApplyBlockElementCommand::splitTextNodes(const VisiblePosition& start, int numParagraphs)
    185 {
    186     VisiblePosition currentParagraphStart = start;
    187     bool hasSplit = false;
    188     int paragraphCount;
    189     for (paragraphCount = 0; paragraphCount < numParagraphs; ++paragraphCount) {
    190         // If there are multiple paragraphs in a single text node, we split the text node into a separate node for each paragraph.
    191         if (currentParagraphStart.deepEquivalent().node()->isTextNode() && currentParagraphStart.deepEquivalent().node() == startOfParagraph(currentParagraphStart.previous()).deepEquivalent().node()) {
    192             Text* textNode = static_cast<Text *>(currentParagraphStart.deepEquivalent().node());
    193             int offset = currentParagraphStart.deepEquivalent().offsetInContainerNode();
    194             splitTextNode(textNode, offset);
    195             currentParagraphStart = VisiblePosition(textNode, 0, VP_DEFAULT_AFFINITY);
    196             hasSplit = true;
    197         }
    198         VisiblePosition nextParagraph = startOfParagraph(endOfParagraph(currentParagraphStart).next());
    199         if (nextParagraph.isNull())
    200             break;
    201         currentParagraphStart = nextParagraph;
    202     }
    203     return hasSplit;
    204 }
    205 
    206 }
     263}

  • trunk/WebCore/editing/ApplyBlockElementCommand.h

    r68972 r69354  
    4747private:
    4848    virtual void doApply();
    49     virtual void formatParagraph(const VisiblePosition& endOfCurrentParagraph, RefPtr<Element>&) = 0;
    50     bool splitTextNodes(const VisiblePosition& start, int numParagraphs);
     49    virtual void formatRange(const Position& start, const Position&, RefPtr<Element>&) = 0;
     50    void rangeForParagraphSplittingTextNodesIfNeeded(const VisiblePosition&, Position&, Position&);
     51    VisiblePosition endOfNextParagrahSplittingTextNodesIfNeeded(VisiblePosition&, Position&, Position&);
    5152
    5253    QualifiedName m_tagName;

  • trunk/WebCore/editing/FormatBlockCommand.cpp

    r68972 r69354  
    4242}
    4343
    44 void FormatBlockCommand::formatParagraph(const VisiblePosition& endOfCurrentParagraph, RefPtr<Element>&)
     44void FormatBlockCommand::formatRange(const Position&, const Position& end, RefPtr<Element>&)
    4545{
    46     setEndingSelection(endOfCurrentParagraph);
     46    setEndingSelection(VisiblePosition(end));
     47
    4748    Node* refNode = enclosingBlockFlowElement(endingSelection().visibleStart());
    4849    if (refNode->hasTagName(tagName()))
    … …  
    5051        return;
    5152
    52     VisiblePosition paragraphStart = startOfParagraph(endingSelection().visibleStart());
    53     VisiblePosition paragraphEnd = endOfParagraph(endingSelection().visibleStart());
     53    VisiblePosition paragraphStart = startOfParagraph(end);
     54    VisiblePosition paragraphEnd = endOfParagraph(end);
    5455    VisiblePosition blockStart = startOfBlock(endingSelection().visibleStart());
    5556    VisiblePosition blockEnd = endOfBlock(endingSelection().visibleStart());

  • trunk/WebCore/editing/FormatBlockCommand.h

    r68972 r69354  
    4242    FormatBlockCommand(Document*, const QualifiedName& tagName);
    4343
    44     virtual void formatParagraph(const VisiblePosition& endOfCurrentParagraph, RefPtr<Element>&);
     44    virtual void formatRange(const Position&, const Position&, RefPtr<Element>&);
    4545    virtual EditAction editingAction() const { return EditActionFormatBlock; }
    4646};

  • trunk/WebCore/editing/IndentOutdentCommand.cpp

    r68972 r69354  
    5757}
    5858
    59 bool IndentOutdentCommand::tryIndentingAsListItem(const VisiblePosition& endOfCurrentParagraph)
     59bool IndentOutdentCommand::tryIndentingAsListItem(const Position& start, const Position& end)
    6060{
    6161    // If our selection is not inside a list, bail out.
    62     Node* lastNodeInSelectedParagraph = endOfCurrentParagraph.deepEquivalent().node();
     62    Node* lastNodeInSelectedParagraph = start.node();
    6363    RefPtr<Element> listNode = enclosingList(lastNodeInSelectedParagraph);
    6464    if (!listNode)
    … …  
    7979    insertNodeBefore(newList, selectedListItem);
    8080
    81     moveParagraphWithClones(startOfParagraph(endOfCurrentParagraph), endOfCurrentParagraph, newList.get(), selectedListItem);
     81    moveParagraphWithClones(start, end, newList.get(), selectedListItem);
    8282
    8383    if (canMergeLists(previousList, newList.get()))
    … …  
    8888    return true;
    8989}
    90    
    91 void IndentOutdentCommand::indentIntoBlockquote(const VisiblePosition& endOfCurrentParagraph, RefPtr<Element>& targetBlockquote)
    92 {
    93     Position start = startOfParagraph(endOfCurrentParagraph).deepEquivalent();
     90
     91void IndentOutdentCommand::indentIntoBlockquote(const Position& start, const Position& end, RefPtr<Element>& targetBlockquote)
     92{
    9493    Node* enclosingCell = enclosingNodeOfType(start, &isTableCell);
    9594    Node* nodeToSplitTo;
    … …  
    110109    }
    111110
    112     moveParagraphWithClones(startOfParagraph(endOfCurrentParagraph), endOfCurrentParagraph, targetBlockquote.get(), outerBlock.get());
     111    moveParagraphWithClones(start, end, targetBlockquote.get(), outerBlock.get());
    113112}
    114113
    … …  
    221220}
    222221
    223 void IndentOutdentCommand::formatParagraph(const VisiblePosition& endOfCurrentParagraph, RefPtr<Element>& blockquoteForNextIndent)
    224 {
    225     if (tryIndentingAsListItem(endOfCurrentParagraph))
     222void IndentOutdentCommand::formatRange(const Position& start, const Position& end, RefPtr<Element>& blockquoteForNextIndent)
     223{
     224    if (tryIndentingAsListItem(start, end))
    226225        blockquoteForNextIndent = 0;
    227226    else
    228         indentIntoBlockquote(endOfCurrentParagraph, blockquoteForNextIndent);
    229 }
    230 
    231 }
     227        indentIntoBlockquote(start, end, blockquoteForNextIndent);
     228}
     229
     230}

  • trunk/WebCore/editing/IndentOutdentCommand.h

    r68972 r69354  
    5050    void outdentRegion(const VisiblePosition&, const VisiblePosition&);
    5151    void outdentParagraph();
    52     bool tryIndentingAsListItem(const VisiblePosition&);
    53     void indentIntoBlockquote(const VisiblePosition&, RefPtr<Element>&);
     52    bool tryIndentingAsListItem(const Position&, const Position&);
     53    void indentIntoBlockquote(const Position&, const Position&, RefPtr<Element>&);
    5454
    5555    void formatSelection(const VisiblePosition& startOfSelection, const VisiblePosition& endOfSelection);
    56     void formatParagraph(const VisiblePosition& endOfCurrentParagraph, RefPtr<Element>& blockquoteForNextIndent);
     56    void formatRange(const Position&, const Position&, RefPtr<Element>& blockquoteForNextIndent);
    5757
    5858    EIndentType m_typeOfAction;
Note: See TracChangeset for help on using the changeset viewer.