Changeset 73411 in webkit


Ignore:
Timestamp:
Dec 6, 2010 4:31:34 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

2010-12-05 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
​https://bugs.webkit.org/show_bug.cgi?id=47300

The bug was caused by FormatBlockCommand::formatRange's not removing refNode when the refNode
contains more than one paragraphs even when the refNode is fully selected.

Fixed the bug by modifying FormatBlockCommand::formatRange to correctly remove the node in
such a situation.

Also fixed a bug in ApplyBlockElementCommand::formatSelection that the end of selection
is not properly updated when the end of selection resides in the node split by
rangeForParagraphSplittingTextNodesIfNeeded or endOfNextParagrahSplittingTextNodesIfNeeded.

Test: editing/execCommand/format-block-multiple-paragraphs-in-pre.html

  • editing/ApplyBlockElementCommand.cpp: (WebCore::ApplyBlockElementCommand::formatSelection): Calls formatRange with m_endOfLastParagraph. (WebCore::ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded): Updates m_endOfLastParagraph when the position points to the node split by this function. (WebCore::ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfNeeded): Ditto.
  • editing/ApplyBlockElementCommand.h: Added m_endOfLastParagraph as a member variable.
  • editing/FormatBlockCommand.cpp: (WebCore::FormatBlockCommand::formatRange): See above.
  • editing/FormatBlockCommand.h:
  • editing/IndentOutdentCommand.cpp: (WebCore::IndentOutdentCommand::formatRange): Ignores the end of selection.
  • editing/IndentOutdentCommand.h:

2010-12-05 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
​https://bugs.webkit.org/show_bug.cgi?id=47300

Added a test to ensure execCommand('FormatBlock') correctly removes pre when formatting paragraphs within.
Also rebaselined several tests because WebKit no longer erroneously format the paragraphs immediately
after the selection.

  • editing/execCommand/format-block-multiple-paragraphs-in-pre-expected.txt: Added.
  • editing/execCommand/format-block-multiple-paragraphs-in-pre.html: Added.
  • editing/execCommand/format-block-multiple-paragraphs-expected.txt: No longer erroneously format the last paragraph in a pre by a blockquote when formatting all but the last paragraph in the pre.
  • editing/execCommand/indent-pre-expected.txt: No longer erroneously format the last paragraph in the pre-list by a blockquote.
Location:
trunk
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r73408 r73411  
     12010-12-05  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
     6        https://bugs.webkit.org/show_bug.cgi?id=47300
     7
     8        Added a test to ensure execCommand('FormatBlock') correctly removes pre when formatting paragraphs within.
     9        Also rebaselined several tests because WebKit no longer erroneously format the paragraphs immediately
     10        after the selection.
     11
     12        * editing/execCommand/format-block-multiple-paragraphs-in-pre-expected.txt: Added.
     13        * editing/execCommand/format-block-multiple-paragraphs-in-pre.html: Added.
     14        * editing/execCommand/format-block-multiple-paragraphs-expected.txt: No longer erroneously format
     15        the last paragraph in a pre by a blockquote when formatting all but the last paragraph in the pre.
     16        * editing/execCommand/indent-pre-expected.txt: No longer erroneously format the last paragraph in
     17        the pre-list by a blockquote.
     18
    1192010-12-06  Victor Wang  <victorw@chromium.org>
    220

  • trunk/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-expected.txt

    r69836 r73411  
    5858| "
    5959"
    60 | <div>
    61 |   <p>
    62 |     "<#selection-anchor>hello"
    63 |     <br>
    64 |     "world<#selection-focus>"
     60| <p>
     61|   "<#selection-anchor>hello"
     62|   <br>
     63|   "world<#selection-focus>"
    6564| "
    6665"
    … …  
    133132|     <br>
    134133|     "world<#selection-focus>"
    135 |     <br>
    136 |     "webkit"
     134|   "webkit
     135"
    137136| "
    138137"

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

    r69354 r73411  
    4747|               <br>
    4848|               "list three"
    49 |               <br>
    50 |               "list four"
     49|             "list four
     50"
    5151|       "
    5252

  • trunk/WebCore/ChangeLog

    r73406 r73411  
     12010-12-05  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
     6        https://bugs.webkit.org/show_bug.cgi?id=47300
     7
     8        The bug was caused by FormatBlockCommand::formatRange's not removing refNode when the refNode
     9        contains more than one paragraphs even when the refNode is fully selected.
     10
     11        Fixed the bug by modifying FormatBlockCommand::formatRange to correctly remove the node in
     12        such a situation.
     13
     14        Also fixed a bug in ApplyBlockElementCommand::formatSelection that the end of selection
     15        is not properly updated when the end of selection resides in the node split by
     16        rangeForParagraphSplittingTextNodesIfNeeded or endOfNextParagrahSplittingTextNodesIfNeeded.
     17
     18        Test: editing/execCommand/format-block-multiple-paragraphs-in-pre.html
     19
     20        * editing/ApplyBlockElementCommand.cpp:
     21        (WebCore::ApplyBlockElementCommand::formatSelection): Calls formatRange with m_endOfLastParagraph.
     22        (WebCore::ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded): Updates
     23        m_endOfLastParagraph when the position points to the node split by this function.
     24        (WebCore::ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfNeeded): Ditto.
     25        * editing/ApplyBlockElementCommand.h: Added m_endOfLastParagraph as a member variable.
     26        * editing/FormatBlockCommand.cpp:
     27        (WebCore::FormatBlockCommand::formatRange): See above.
     28        * editing/FormatBlockCommand.h:
     29        * editing/IndentOutdentCommand.cpp:
     30        (WebCore::IndentOutdentCommand::formatRange): Ignores the end of selection.
     31        * editing/IndentOutdentCommand.h:
     32
    1332010-12-03  Zhenyao Mo  <zmo@google.com>
    234

  • trunk/WebCore/editing/ApplyBlockElementCommand.cpp

    r69836 r73411  
    110110    VisiblePosition endOfCurrentParagraph = endOfParagraph(startOfSelection);
    111111    VisiblePosition endAfterSelection = endOfParagraph(endOfParagraph(endOfSelection).next());
    112     VisiblePosition endOfLastParagraph = endOfParagraph(endOfSelection);
     112    m_endOfLastParagraph = endOfParagraph(endOfSelection).deepEquivalent();
    113113
    114114    bool atEnd = false;
    115115    Position end;
    116116    while (endOfCurrentParagraph != endAfterSelection && !atEnd) {
    117         if (endOfCurrentParagraph == endOfLastParagraph)
     117        if (endOfCurrentParagraph.deepEquivalent() == m_endOfLastParagraph)
    118118            atEnd = true;
    119119
    … …  
    125125        VisiblePosition endOfNextParagraph = endOfNextParagrahSplittingTextNodesIfNeeded(endOfCurrentParagraph, start, end);
    126126
    127         formatRange(start, end, blockquoteForNextIndent);
     127        formatRange(start, end, m_endOfLastParagraph, blockquoteForNextIndent);
    128128
    129129        // Don't put the next paragraph in the blockquote we just created for this paragraph unless
    … …  
    181181
    182182    RenderStyle* startStyle = renderStyleOfEnclosingTextNode(start);
     183    bool isStartAndEndOnSameNode = false;
    183184    if (startStyle) {
     185        isStartAndEndOnSameNode = renderStyleOfEnclosingTextNode(end) && start.node() == end.node();
     186        bool isStartAndEndOfLastParagraphOnSameNode = renderStyleOfEnclosingTextNode(m_endOfLastParagraph) && start.node() == m_endOfLastParagraph.node();
     187
    184188        // Avoid obtanining the start of next paragraph for start
    185189        if (startStyle->preserveNewline() && isNewLineAtPosition(start) && !isNewLineAtPosition(start.previous()) && start.offsetInContainerNode() > 0)
    … …  
    191195            splitTextNode(static_cast<Text*>(start.node()), startOffset);
    192196            start = positionBeforeNode(start.node());
    193             if (start.node() == end.node()) {
     197            if (isStartAndEndOnSameNode) {
    194198                ASSERT(end.offsetInContainerNode() >= startOffset);
    195199                end = Position(end.node(), end.offsetInContainerNode() - startOffset, Position::PositionIsOffsetInAnchor);
    196200            }
     201            if (isStartAndEndOfLastParagraphOnSameNode) {
     202                ASSERT(m_endOfLastParagraph.offsetInContainerNode() >= startOffset);
     203                m_endOfLastParagraph = Position(m_endOfLastParagraph.node(), m_endOfLastParagraph.offsetInContainerNode() - startOffset,
     204                    Position::PositionIsOffsetInAnchor);
     205            }
    197206        }
    198207    }
    … …  
    200209    RenderStyle* endStyle = renderStyleOfEnclosingTextNode(end);
    201210    if (endStyle) {
     211        bool isEndAndEndOfLastParagraphOnSameNode = renderStyleOfEnclosingTextNode(m_endOfLastParagraph) && end.node() == m_endOfLastParagraph.node();
    202212        // Include \n at the end of line if we're at an empty paragraph
    203213        if (endStyle->preserveNewline() && start == end
    … …  
    206216            if (!isNewLineAtPosition(end.previous()) && isNewLineAtPosition(end))
    207217                end = Position(end.node(), endOffset + 1, Position::PositionIsOffsetInAnchor);
     218            if (isEndAndEndOfLastParagraphOnSameNode && end.offsetInContainerNode() >= m_endOfLastParagraph.offsetInContainerNode())
     219                m_endOfLastParagraph = end;
    208220        }
    209221
    … …  
    212224            && end.offsetInContainerNode() < end.containerNode()->maxCharacterOffset()) {
    213225            splitTextNode(static_cast<Text*>(end.node()), end.offsetInContainerNode());
    214             if (start.node() == end.node())
     226            if (isStartAndEndOnSameNode)
    215227                start = positionBeforeNode(end.node()->previousSibling());
     228            if (isEndAndEndOfLastParagraphOnSameNode) {
     229                if (m_endOfLastParagraph.offsetInContainerNode() == end.offsetInContainerNode())
     230                    m_endOfLastParagraph = lastPositionInNode(end.node()->previousSibling());
     231                else
     232                    m_endOfLastParagraph = Position(end.node(), m_endOfLastParagraph.offsetInContainerNode() - end.offsetInContainerNode(),
     233                                                    Position::PositionIsOffsetInAnchor);
     234            }
    216235            end = lastPositionInNode(end.node()->previousSibling());
    217236        }
    … …  
    244263        ASSERT(end.offsetInContainerNode() < position.offsetInContainerNode());
    245264        end = Position(containerNode->previousSibling(), end.offsetInContainerNode(), Position::PositionIsOffsetInAnchor);
     265    }
     266    if (m_endOfLastParagraph.anchorType() == Position::PositionIsOffsetInAnchor && containerNode.get() == m_endOfLastParagraph.containerNode()) {
     267        if (m_endOfLastParagraph.offsetInContainerNode() < position.offsetInContainerNode())
     268            m_endOfLastParagraph = Position(containerNode->previousSibling(), m_endOfLastParagraph.offsetInContainerNode(), Position::PositionIsOffsetInAnchor);
     269        else
     270            m_endOfLastParagraph = Position(containerNode, m_endOfLastParagraph.offsetInContainerNode() - 1, Position::PositionIsOffsetInAnchor);
    246271    }
    247272

  • trunk/WebCore/editing/ApplyBlockElementCommand.h

    r69354 r73411  
    4747private:
    4848    virtual void doApply();
    49     virtual void formatRange(const Position& start, const Position&, RefPtr<Element>&) = 0;
     49    virtual void formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>&) = 0;
    5050    void rangeForParagraphSplittingTextNodesIfNeeded(const VisiblePosition&, Position&, Position&);
    5151    VisiblePosition endOfNextParagrahSplittingTextNodesIfNeeded(VisiblePosition&, Position&, Position&);
    … …  
    5454    AtomicString m_className;
    5555    AtomicString m_inlineStyle;
     56    Position m_endOfLastParagraph;
    5657};
    5758

  • trunk/WebCore/editing/FormatBlockCommand.cpp

    r69876 r73411  
    5959}
    6060
    61 void FormatBlockCommand::formatRange(const Position& start, const Position& end, RefPtr<Element>& blockNode)
     61void FormatBlockCommand::formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>& blockNode)
    6262{
    6363    Node* nodeToSplitTo = enclosingBlockToSplitTreeTo(start.node());
    … …  
    6565    RefPtr<Node> nodeAfterInsertionPosition = outerBlock;
    6666
     67    RefPtr<Range> range = Range::create(document(), start, endOfSelection);
    6768    Element* refNode = enclosingBlockFlowElement(end);
    6869    Element* root = editableRootForPosition(start);
    69     if (isElementForFormatBlock(refNode->tagQName()) && start == startOfBlock(start) && end == endOfBlock(end)
     70    if (isElementForFormatBlock(refNode->tagQName()) && start == startOfBlock(start)
     71        && (end == endOfBlock(end) || isNodeVisiblyContainedWithin(refNode, range.get()))
    7072        && refNode != root && !root->isDescendantOf(refNode)) {
    7173        // Already in a block element that only contains the current paragraph

  • trunk/WebCore/editing/FormatBlockCommand.h

    r69876 r73411  
    4646
    4747    void formatSelection(const VisiblePosition& startOfSelection, const VisiblePosition& endOfSelection);
    48     void formatRange(const Position&, const Position&, RefPtr<Element>&);
     48    void formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>&);
    4949    EditAction editingAction() const { return EditActionFormatBlock; }
    5050

  • trunk/WebCore/editing/IndentOutdentCommand.cpp

    r70594 r73411  
    223223}
    224224
    225 void IndentOutdentCommand::formatRange(const Position& start, const Position& end, RefPtr<Element>& blockquoteForNextIndent)
     225void IndentOutdentCommand::formatRange(const Position& start, const Position& end, const Position&, RefPtr<Element>& blockquoteForNextIndent)
    226226{
    227227    if (tryIndentingAsListItem(start, end))

  • trunk/WebCore/editing/IndentOutdentCommand.h

    r69354 r73411  
    5454
    5555    void formatSelection(const VisiblePosition& startOfSelection, const VisiblePosition& endOfSelection);
    56     void formatRange(const Position&, const Position&, RefPtr<Element>& blockquoteForNextIndent);
     56    void formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>& blockquoteForNextIndent);
    5757
    5858    EIndentType m_typeOfAction;
Note: See TracChangeset for help on using the changeset viewer.