Changeset 69836 in webkit


Ignore:
Timestamp:
Oct 14, 2010 9:09:48 PM (14 years ago)
Author:
rniwa@webkit.org
Message:

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

Reviewed by Tony Chang and Darin Adler.

execCommand FormatBlock creates lots of blockquotes
​https://bugs.webkit.org/show_bug.cgi?id=19795

The bug was caused by WebKit's not reusing the block node added by previous iteration
and its inserting block node at wrong places.

Fixed the bug by rewriting FormatBlockCommand::formatRange. New code resembles that of
IndentOutdentCommand::indentIntoBlockquote. The difference between two is that formatRange
avoids the existing block elements when replacing blocks and it also adds a placeholder
when removing the existing block caused paragraphs to collapse.

Also fixed a bug in moveParagraphWithClones where erroneous br is added to the start of
the block element to which the paragraph is moved if the block element is the start of a paragraph
and not the end of a paragraph.

Tests: editing/execCommand/format-block-multiple-paragraphs.html

editing/execCommand/format-block-table.html

  • editing/CompositeEditCommand.cpp: (WebCore::CompositeEditCommand::moveParagraphWithClones): No longer adds erroneous br.
  • editing/EditorCommand.cpp: (WebCore::executeFormatBlock):
  • editing/FormatBlockCommand.cpp: (WebCore::FormatBlockCommand::formatRange): Rewritten; see above. (WebCore::FormatBlockCommand::isElementToApplyInFormatBlockCommand): Renamed from validBlockElement and moved from htmlediting.cpp. (WebCore::enclosingBlockToSplitTreeTo): Added.
  • editing/FormatBlockCommand.h:
  • editing/VisiblePosition.cpp: (WebCore::enclosingBlockFlowElement): Changed the return type to Element*
  • editing/VisiblePosition.h:

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

Reviewed by Tony Chang and Darin Adler.

execCommand FormatBlock creates lots of blockquotes
​https://bugs.webkit.org/show_bug.cgi?id=19795

Added tests to ensure WebKit does not add multiple block elements when applying block element to
multiple paragraphs. Also added a test to ensure formatBlock works with tables.

  • fast/html/nav-element-expected.txt: Preserved new lines and removed redundant br.
  • editing/execCommand/format-block-expected.txt: Preserved span and removed erroneous br.
  • editing/execCommand/format-block-from-range-selection-expected.txt: Merged dl's and removed erroneous br.
  • editing/execCommand/format-block-multiple-paragraphs-expected.txt: Added.
  • editing/execCommand/format-block-multiple-paragraphs.html: Added.
  • editing/execCommand/format-block-table-expected.txt: Added.
  • editing/execCommand/format-block-table.html: Added.
  • editing/execCommand/format-block-with-braces-expected.txt: Removed erroneous br.
Location:
trunk
Files:
4 added
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r69835 r69836  
     12010-10-14  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang and Darin Adler.
     4
     5        execCommand FormatBlock creates lots of blockquotes
     6        https://bugs.webkit.org/show_bug.cgi?id=19795
     7
     8        Added tests to ensure WebKit does not add multiple block elements when applying block element to
     9        multiple paragraphs.  Also added a test to ensure formatBlock works with tables.
     10
     11        * fast/html/nav-element-expected.txt: Preserved new lines and removed redundant br.
     12        * editing/execCommand/format-block-expected.txt: Preserved span and removed erroneous br.
     13        * editing/execCommand/format-block-from-range-selection-expected.txt: Merged dl's and removed erroneous br.
     14        * editing/execCommand/format-block-multiple-paragraphs-expected.txt: Added.
     15        * editing/execCommand/format-block-multiple-paragraphs.html: Added.
     16        * editing/execCommand/format-block-table-expected.txt: Added.
     17        * editing/execCommand/format-block-table.html: Added.
     18        * editing/execCommand/format-block-with-braces-expected.txt: Removed erroneous br.
     19
    1202010-10-14  Kinuko Yasuda  <kinuko@chromium.org>
    221

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

    r69640 r69836  
    5353| <pre>
    5454|   "Make Pre"
    55 |   <br>
    5655| "
    5756"
    … …  
    6362|   <br>
    6463|   <h1>
    65 |     "Make h1"
    66 |     <br>
     64|     <span>
     65|       id="item2"
     66|       "Make h1"
    6767|   "baz"
    6868| "

  • trunk/LayoutTests/editing/execCommand/format-block-from-range-selection-expected.txt

    r69640 r69836  
    3333| <dl>
    3434|   "Fo<#selection-anchor>o"
     35|   <br>
     36|   "bar"
     37|   <br>
     38|   <span>
     39|     "baz"
     40|     <br>
     41|   "raz"
     42|   <br>
     43|   "
     44dar"
     45|   <br>
     46|   "
     47"
     48|   "yar<#selection-focus>"
    3549| "
    3650"
    37 | <div>
    38 |   <dl>
    39 |     "bar"
    40 |   <span>
    41 |     <dl>
    42 |       "baz"
    43 |   <dl>
    44 |     "raz"
    45 | <dl>
    46 |   "dar"
    47 |   " "
    4851| "
    4952"
    50 | <dl>
    51 |   "ya<#selection-focus>r"
    5253| "
    5354"

  • trunk/LayoutTests/editing/execCommand/format-block-with-braces-expected.txt

    r69640 r69836  
    1717After FormatBlock with <h1>:
    1818| <h1>
    19 |   "<#selection-caret>Format Me"
    20 |   <br>
     19|   "
     20<#selection-caret>Format Me
     21"

  • trunk/LayoutTests/fast/html/nav-element-expected.txt

    r47489 r69836  
    2525<p><br></p>
    2626
    27 <div contenteditable="true" id="editable"><nav>Test of FormatBlock behavior. This text should have a green border.<br></nav></div>
     27<div contenteditable="true" id="editable"><nav>
     28Test of FormatBlock behavior. This text should have a green border.
     29</nav></div>
    2830

  • trunk/WebCore/ChangeLog

    r69831 r69836  
     12010-10-14  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang and Darin Adler.
     4
     5        execCommand FormatBlock creates lots of blockquotes
     6        https://bugs.webkit.org/show_bug.cgi?id=19795
     7
     8        The bug was caused by WebKit's not reusing the block node added by previous iteration
     9        and its inserting block node at wrong places.
     10
     11        Fixed the bug by rewriting FormatBlockCommand::formatRange.  New code resembles that of
     12        IndentOutdentCommand::indentIntoBlockquote.  The difference between two is that formatRange
     13        avoids the existing block elements when replacing blocks and it also adds a placeholder
     14        when removing the existing block caused paragraphs to collapse.
     15
     16        Also fixed a bug in moveParagraphWithClones where erroneous br is added to the start of
     17        the block element to which the paragraph is moved if the block element is the start of a paragraph
     18        and not the end of a paragraph.
     19
     20        Tests: editing/execCommand/format-block-multiple-paragraphs.html
     21               editing/execCommand/format-block-table.html
     22
     23        * editing/CompositeEditCommand.cpp:
     24        (WebCore::CompositeEditCommand::moveParagraphWithClones): No longer adds erroneous br.
     25        * editing/EditorCommand.cpp:
     26        (WebCore::executeFormatBlock):
     27        * editing/FormatBlockCommand.cpp:
     28        (WebCore::FormatBlockCommand::formatRange): Rewritten; see above.
     29        (WebCore::FormatBlockCommand::isElementToApplyInFormatBlockCommand): Renamed from validBlockElement
     30        and moved from htmlediting.cpp.
     31        (WebCore::enclosingBlockToSplitTreeTo): Added.
     32        * editing/FormatBlockCommand.h:
     33        * editing/VisiblePosition.cpp:
     34        (WebCore::enclosingBlockFlowElement): Changed the return type to Element*
     35        * editing/VisiblePosition.h:
     36
    1372010-10-14  Justin Schuh  <jschuh@chromium.org>
    238

  • trunk/WebCore/editing/ApplyBlockElementCommand.cpp

    r69640 r69836  
    113113
    114114    bool atEnd = false;
     115    Position end;
    115116    while (endOfCurrentParagraph != endAfterSelection && !atEnd) {
    116         Position start;
    117         Position end;
    118 
    119117        if (endOfCurrentParagraph == endOfLastParagraph)
    120118            atEnd = true;

  • trunk/WebCore/editing/CompositeEditCommand.cpp

    r69437 r69836  
    864864    afterParagraph = VisiblePosition(afterParagraph.deepEquivalent());
    865865
    866     if (beforeParagraph.isNotNull() && !isTableElement(beforeParagraph.deepEquivalent().node()) && (!isEndOfParagraph(beforeParagraph) || beforeParagraph == afterParagraph)) {
     866    if (beforeParagraph.isNotNull() && !isTableElement(beforeParagraph.deepEquivalent().node())
     867        && ((!isEndOfParagraph(beforeParagraph) && !isStartOfParagraph(beforeParagraph)) || beforeParagraph == afterParagraph)) {
    867868        // FIXME: Trim text between beforeParagraph and afterParagraph if they aren't equal.
    868869        insertNodeAt(createBreakElement(document()), beforeParagraph.deepEquivalent());

  • trunk/WebCore/editing/EditorCommand.cpp

    r69640 r69836  
    433433    if (tagName[0] == '<' && tagName[tagName.length() - 1] == '>')
    434434        tagName = tagName.substring(1, tagName.length() - 2);
    435     if (!validBlockTag(tagName))
    436         return false;
    437435
    438436    ExceptionCode ec;
    … …  
    441439        return false;
    442440    QualifiedName qualifiedTagName(prefix, localName, xhtmlNamespaceURI);
     441
     442    if (!FormatBlockCommand::isElementToApplyInFormatBlockCommand(qualifiedTagName))
     443        return false;
    443444
    444445    applyCommand(FormatBlockCommand::create(frame->document(), qualifiedTagName));

  • trunk/WebCore/editing/FormatBlockCommand.cpp

    r69640 r69836  
    3737using namespace HTMLNames;
    3838
     39static Node* enclosingBlockToSplitTreeTo(Node* startNode);
     40
    3941FormatBlockCommand::FormatBlockCommand(Document* document, const QualifiedName& tagName)
    4042    : ApplyBlockElementCommand(document, tagName)
    … …  
    4244}
    4345
    44 void FormatBlockCommand::formatRange(const Position&, const Position& end, RefPtr<Element>&)
     46void FormatBlockCommand::formatRange(const Position& start, const Position& end, RefPtr<Element>& blockNode)
    4547{
    46     setEndingSelection(VisiblePosition(end));
     48    Node* nodeToSplitTo = enclosingBlockToSplitTreeTo(start.node());
     49    RefPtr<Node> outerBlock = (start.node() == nodeToSplitTo) ? start.node() : splitTreeToNode(start.node(), nodeToSplitTo);
     50    RefPtr<Node> nodeAfterInsertionPosition = outerBlock;
    4751
    48     Node* refNode = enclosingBlockFlowElement(endingSelection().visibleStart());
    49     if (refNode->hasTagName(tagName()))
    50         // We're already in a block with the format we want, so we don't have to do anything
    51         return;
     52    Element* refNode = enclosingBlockFlowElement(end);
     53    Element* root = editableRootForPosition(start);
     54    if (isElementToApplyInFormatBlockCommand(refNode->tagQName()) && start == startOfBlock(start) && end == endOfBlock(end)
     55        && refNode != root && !root->isDescendantOf(refNode)) {
     56        // Already in a block element that only contains the current paragraph
     57        if (refNode->hasTagName(tagName()))
     58            return;
     59        nodeAfterInsertionPosition = refNode;
     60    }
    5261
    53     VisiblePosition paragraphStart = startOfParagraph(end);
    54     VisiblePosition paragraphEnd = endOfParagraph(end);
    55     VisiblePosition blockStart = startOfBlock(endingSelection().visibleStart());
    56     VisiblePosition blockEnd = endOfBlock(endingSelection().visibleStart());
    57     RefPtr<Element> blockNode = createBlockElement();
    58     RefPtr<Element> placeholder = createBreakElement(document());
     62    if (!blockNode) {
     63        // Create a new blockquote and insert it as a child of the root editable element. We accomplish
     64        // this by splitting all parents of the current paragraph up to that point.
     65        blockNode = createBlockElement();
     66        insertNodeBefore(blockNode, nodeAfterInsertionPosition);
     67    }
    5968
    60     Node* root = endingSelection().start().node()->rootEditableElement();
    61     if (validBlockTag(refNode->nodeName().lower()) &&
    62         paragraphStart == blockStart && paragraphEnd == blockEnd &&
    63         refNode != root && !root->isDescendantOf(refNode))
    64         // Already in a valid block tag that only contains the current paragraph, so we can swap with the new tag
    65         insertNodeBefore(blockNode, refNode);
    66     else {
    67         // Avoid inserting inside inline elements that surround paragraphStart with upstream().
    68         // This is only to avoid creating bloated markup.
    69         insertNodeAt(blockNode, paragraphStart.deepEquivalent().upstream());
     69    Position lastParagraphInBlockNode = lastPositionInNode(blockNode.get());
     70    bool wasEndOfParagraph = isEndOfParagraph(lastParagraphInBlockNode);
     71
     72    moveParagraphWithClones(start, end, blockNode.get(), outerBlock.get());
     73
     74    if (wasEndOfParagraph && !isEndOfParagraph(lastParagraphInBlockNode) && !isStartOfParagraph(lastParagraphInBlockNode))
     75        insertBlockPlaceholder(lastParagraphInBlockNode);
     76}
     77
     78// FIXME: We should consider merging this function with isElementForFormatBlockCommand in Editor.cpp
     79// Checks if a tag name is valid for execCommand('FormatBlock').
     80bool FormatBlockCommand::isElementToApplyInFormatBlockCommand(const QualifiedName& tagName)
     81{
     82    DEFINE_STATIC_LOCAL(HashSet<QualifiedName>, blockTags, ());
     83    if (blockTags.isEmpty()) {
     84        blockTags.add(addressTag);
     85        blockTags.add(articleTag);
     86        blockTags.add(asideTag);
     87        blockTags.add(blockquoteTag);
     88        blockTags.add(ddTag);
     89        blockTags.add(divTag);
     90        blockTags.add(dlTag);
     91        blockTags.add(dtTag);
     92        blockTags.add(footerTag);
     93        blockTags.add(h1Tag);
     94        blockTags.add(h2Tag);
     95        blockTags.add(h3Tag);
     96        blockTags.add(h4Tag);
     97        blockTags.add(h5Tag);
     98        blockTags.add(h6Tag);
     99        blockTags.add(headerTag);
     100        blockTags.add(hgroupTag);
     101        blockTags.add(navTag);
     102        blockTags.add(pTag);
     103        blockTags.add(preTag);
     104        blockTags.add(sectionTag);
    70105    }
    71     appendNode(placeholder, blockNode);
     106    return blockTags.contains(tagName);
     107}
    72108
    73     VisiblePosition destination(Position(placeholder.get(), 0));
    74     if (paragraphStart == paragraphEnd && !lineBreakExistsAtVisiblePosition(paragraphStart)) {
    75         setEndingSelection(destination);
    76         return;
     109Node* enclosingBlockToSplitTreeTo(Node* startNode)
     110{
     111    Node* lastBlock = startNode;
     112    for (Node* n = startNode; n; n = n->parentNode()) {
     113        if (!n->isContentEditable())
     114            return lastBlock;
     115        if (isTableCell(n) || n->hasTagName(bodyTag) || !n->parentNode() || !n->parentNode()->isContentEditable()
     116            || (n->isElementNode() && FormatBlockCommand::isElementToApplyInFormatBlockCommand(static_cast<Element*>(n)->tagQName())))
     117            return n;
     118        if (isBlock(n))
     119            lastBlock = n;
     120        if (isListElement(n))
     121            return n->parentNode()->isContentEditable() ? n->parentNode() : n;
    77122    }
    78     moveParagraph(paragraphStart, paragraphEnd, destination, true, false);
     123    return lastBlock;
    79124}
    80125

  • trunk/WebCore/editing/FormatBlockCommand.h

    r69640 r69836  
    3939    }
    4040
     41    static bool isElementToApplyInFormatBlockCommand(const QualifiedName& tagName);
     42
    4143private:
    4244    FormatBlockCommand(Document*, const QualifiedName& tagName);
    4345
    44     virtual void formatRange(const Position&, const Position&, RefPtr<Element>&);
    45     virtual EditAction editingAction() const { return EditActionFormatBlock; }
     46    void formatRange(const Position&, const Position&, RefPtr<Element>&);
     47    EditAction editingAction() const { return EditActionFormatBlock; }
    4648};
    4749

  • trunk/WebCore/editing/VisiblePosition.cpp

    r69640 r69836  
    635635}
    636636
    637 Node *enclosingBlockFlowElement(const VisiblePosition &visiblePosition)
     637Element* enclosingBlockFlowElement(const VisiblePosition &visiblePosition)
    638638{
    639639    if (visiblePosition.isNull())

  • trunk/WebCore/editing/VisiblePosition.h

    r69640 r69836  
    137137VisiblePosition endVisiblePosition(const Range*, EAffinity);
    138138
    139 Node *enclosingBlockFlowElement(const VisiblePosition&);
     139Element* enclosingBlockFlowElement(const VisiblePosition&);
    140140
    141141bool isFirstVisiblePositionInNode(const VisiblePosition&, const Node*);

  • trunk/WebCore/editing/htmlediting.cpp

    r69640 r69836  
    466466}
    467467
    468 // Checks if a string is a valid tag for the FormatBlockCommand function of execCommand. Expects lower case strings.
    469 bool validBlockTag(const AtomicString& blockTag)
    470 {
    471     if (blockTag.isEmpty())
    472         return false;
    473 
    474     DEFINE_STATIC_LOCAL(HashSet<AtomicString>, blockTags, ());
    475     if (blockTags.isEmpty()) {
    476         blockTags.add(addressTag.localName());
    477         blockTags.add(articleTag.localName());
    478         blockTags.add(asideTag.localName());
    479         blockTags.add(blockquoteTag.localName());
    480         blockTags.add(ddTag.localName());
    481         blockTags.add(divTag.localName());
    482         blockTags.add(dlTag.localName());
    483         blockTags.add(dtTag.localName());
    484         blockTags.add(footerTag.localName());
    485         blockTags.add(h1Tag.localName());
    486         blockTags.add(h2Tag.localName());
    487         blockTags.add(h3Tag.localName());
    488         blockTags.add(h4Tag.localName());
    489         blockTags.add(h5Tag.localName());
    490         blockTags.add(h6Tag.localName());
    491         blockTags.add(headerTag.localName());
    492         blockTags.add(hgroupTag.localName());
    493         blockTags.add(navTag.localName());
    494         blockTags.add(pTag.localName());
    495         blockTags.add(preTag.localName());
    496         blockTags.add(sectionTag.localName());
    497     }
    498     return blockTags.contains(blockTag);
    499 }
    500 
    501468static Node* firstInSpecialElement(const Position& pos)
    502469{

  • trunk/WebCore/editing/htmlediting.h

    r69640 r69836  
    229229String stringWithRebalancedWhitespace(const String&, bool, bool);
    230230const String& nonBreakingSpaceString();
    231 bool validBlockTag(const AtomicString&);
    232    
    233231
    234232}
Note: See TracChangeset for help on using the changeset viewer.