Changeset 14143 in webkit


Ignore:
Timestamp:
May 1, 2006 2:43:03 PM (18 years ago)
Author:
justing
Message:

LayoutTests:

Reviewed by darin


<​http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
Remove a use of hasMoreThanOneBlock

Deleting didn't merge because of code that stopped the merge whenever
the end of the selection to delete was in a fully selected line.

  • editing/deleting/merge-endOfParagraph-expected.checksum: Added.
  • editing/deleting/merge-endOfParagraph-expected.png: Added.
  • editing/deleting/merge-endOfParagraph-expected.txt: Added.
  • editing/deleting/merge-endOfParagraph.html: Added.


Two testcases where paste did not request a merge from deletion, but should have.

  • editing/pasteboard/merge-after-delete-1-expected.checksum: Added.
  • editing/pasteboard/merge-after-delete-1-expected.png: Added.
  • editing/pasteboard/merge-after-delete-1-expected.txt: Added.
  • editing/pasteboard/merge-after-delete-1.html: Added.
  • editing/pasteboard/merge-after-delete-2-expected.checksum: Added.
  • editing/pasteboard/merge-after-delete-2-expected.png: Added.
  • editing/pasteboard/merge-after-delete-2-expected.txt: Added.
  • editing/pasteboard/merge-after-delete-2.html: Added.


Code that prevents nesting incoming blocks in the block being pasted into
could reverse the order of pasted paragraphs.

  • editing/pasteboard/prevent-block-nesting-01-expected.checksum: Added.
  • editing/pasteboard/prevent-block-nesting-01-expected.png: Added.
  • editing/pasteboard/prevent-block-nesting-01-expected.txt: Added.
  • editing/pasteboard/prevent-block-nesting-01.html: Added.

WebCore:

Reviewed by darin


<​http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
Remove a use of hasMoreThanOneBlock, which uses info from the test rendering.

  • editing/DeleteSelectionCommand.cpp: (WebCore::DeleteSelectionCommand::initializePositionData): Removed code that stopped the merge if the end of the selection to delete was in a fully selected line, which was nonsense.


(WebCore::DeleteSelectionCommand::mergeParagraphs):
Deletion does a bad job of updating the endpoints of the selection as it removes
content. If the endpoints have been flip flipped, bail.
If deletion has removed everything from the block that contained the
start of the selection to delete, we can't create a visible position inside
that block to serve as a destination for the merge. So, we insert a placeholder
at that position to prop the block open to let content in.


  • editing/ReplaceSelectionCommand.cpp: (WebCore::ReplaceSelectionCommand::doApply): Added an assert and two early returns for cases where we'll crash. Removed a use of !fragment.hasMoreThanOneBlock, which uses test rendering info and which was wrong. If we've already inserted content during the start merge, insertionPos will be the position just after that content, so inserting new content before insertionPos will reverse its order.
Location:
trunk
Files:
16 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r14106 r14143  
     12006-04-28  Justin Garcia  <justin.garcia@apple.com>
     2
     3        Reviewed by darin
     4       
     5        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
     6        Remove a use of hasMoreThanOneBlock
     7
     8        Deleting didn't merge because of code that stopped the merge whenever
     9        the end of the selection to delete was in a fully selected line.
     10        * editing/deleting/merge-endOfParagraph-expected.checksum: Added.
     11        * editing/deleting/merge-endOfParagraph-expected.png: Added.
     12        * editing/deleting/merge-endOfParagraph-expected.txt: Added.
     13        * editing/deleting/merge-endOfParagraph.html: Added.
     14       
     15        Two testcases where paste did not request a merge from deletion, but should have.
     16        * editing/pasteboard/merge-after-delete-1-expected.checksum: Added.
     17        * editing/pasteboard/merge-after-delete-1-expected.png: Added.
     18        * editing/pasteboard/merge-after-delete-1-expected.txt: Added.
     19        * editing/pasteboard/merge-after-delete-1.html: Added.
     20        * editing/pasteboard/merge-after-delete-2-expected.checksum: Added.
     21        * editing/pasteboard/merge-after-delete-2-expected.png: Added.
     22        * editing/pasteboard/merge-after-delete-2-expected.txt: Added.
     23        * editing/pasteboard/merge-after-delete-2.html: Added.
     24       
     25        Code that prevents nesting incoming blocks in the block being pasted into
     26        could reverse the order of pasted paragraphs.
     27        * editing/pasteboard/prevent-block-nesting-01-expected.checksum: Added.
     28        * editing/pasteboard/prevent-block-nesting-01-expected.png: Added.
     29        * editing/pasteboard/prevent-block-nesting-01-expected.txt: Added.
     30        * editing/pasteboard/prevent-block-nesting-01.html: Added.
     31
    1322006-04-28  Alexey Proskuryakov  <ap@nypop.com>
    233

  • trunk/WebCore/ChangeLog

    r14142 r14143  
     12006-05-01  Justin Garcia  <justin.garcia@apple.com>
     2
     3        Reviewed by darin
     4       
     5        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
     6        Remove a use of hasMoreThanOneBlock, which uses info from the test rendering.
     7
     8        * editing/DeleteSelectionCommand.cpp:
     9        (WebCore::DeleteSelectionCommand::initializePositionData):
     10        Removed code that stopped the merge if the end of the selection to delete
     11        was in a fully selected line, which was nonsense.
     12       
     13        (WebCore::DeleteSelectionCommand::mergeParagraphs):
     14        Deletion does a bad job of updating the endpoints of the selection as it removes
     15        content.  If the endpoints have been flip flipped, bail.
     16        If deletion has removed everything from the block that contained the
     17        start of the selection to delete, we can't create a visible position inside
     18        that block to serve as a destination for the merge.  So, we insert a placeholder
     19        at that position to prop the block open to let content in.
     20       
     21        * editing/ReplaceSelectionCommand.cpp:
     22        (WebCore::ReplaceSelectionCommand::doApply):
     23        Added an assert and two early returns for cases where we'll crash.
     24        Removed a use of !fragment.hasMoreThanOneBlock, which uses test rendering info
     25        and which was wrong.
     26        If we've already inserted content during the start merge, insertionPos will be
     27        the position just after that content, so inserting new content before insertionPos
     28        will reverse its order.
     29
    1302006-05-01  Mitz Pettel  <opendarwin.org@mitzpettel.com>
    231

  • trunk/WebCore/editing/DeleteSelectionCommand.cpp

    r13868 r14143  
    198198    m_endBlock = m_upstreamEnd.node()->enclosingBlockFlowElement();
    199199    m_startNode = m_upstreamStart.node();
    200 
    201     //
    202     // Handle detecting if the line containing the selection end is itself fully selected.
    203     // This is one of the tests that determines if block merging of content needs to be done.
    204     //
    205     VisiblePosition visibleEnd(m_downstreamEnd, VP_DEFAULT_AFFINITY);
    206     if (isEndOfParagraph(visibleEnd)) {
    207         Position previousLineStart = previousLinePosition(visibleEnd, 0).deepEquivalent();
    208         if (previousLineStart.isNull() || Range::compareBoundaryPoints(previousLineStart, m_downstreamStart) >= 0)
    209             m_mergeBlocksAfterDelete = false;
    210     }
    211200
    212201    debugPosition("m_upstreamStart      ", m_upstreamStart);
    … …  
    500489         return;
    501490         
    502     // Do not move content between parts of a table.
     491    // FIXME: The deletion algorithm shouldn't let this happen.
     492    if (Range::compareBoundaryPoints(m_upstreamStart, m_downstreamEnd) > 0)
     493        return;
     494       
     495    // FIXME: Merging will always be unnecessary in this case, but we really bail here because this is a case where
     496    // deletion commonly fails to adjust its endpoints, which would cause the visible position comparison below to false negative.
     497    if (m_endBlock == m_startBlock)
     498        return;
     499       
     500    // Don't move content between parts of a table.
    503501    if (isTableStructureNode(m_downstreamEnd.node()->enclosingBlockFlowElement()) || isTableStructureNode(m_upstreamStart.node()->enclosingBlockFlowElement()))
    504502        return;
    … …  
    507505    VisiblePosition mergeDestination(m_upstreamStart);
    508506   
     507    // We need to merge into m_upstreamStart's block, but it's been emptied out and collapsed by deletion.
     508    if (!mergeDestination.deepEquivalent().node()->isAncestor(m_upstreamStart.node()->enclosingBlockFlowElement())) {
     509        insertNodeAt(createBreakElement(document()).get(), m_upstreamStart.node(), m_upstreamStart.offset());
     510        mergeDestination = VisiblePosition(m_upstreamStart);
     511    }
     512   
    509513    if (mergeDestination == startOfParagraphToMove)
    510         return;
    511    
    512     // FIXME: The above early return should be all we need.
    513     if (m_endBlock == m_startBlock)
    514514        return;
    515515       

  • trunk/WebCore/editing/ReplaceSelectionCommand.cpp

    r14086 r14143  
    507507    Selection selection = endingSelection();
    508508    ASSERT(selection.isCaretOrRange());
     509    ASSERT(selection.start().node());
     510    if (selection.isNone() || !selection.start().node())
     511        return;
    509512   
    510513    if (!selection.isContentRichlyEditable())
    … …  
    535538    // delete the current range selection, or insert paragraph for caret selection, as needed
    536539    if (selection.isRange()) {
    537         bool mergeBlocksAfterDelete = !(fragment.hasInterchangeNewlineAtStart() || fragment.hasInterchangeNewlineAtEnd() || fragment.hasMoreThanOneBlock());
     540        // When the end of the selection to delete is at the end of a paragraph, and the selection
     541        // to delete spans multiple blocks, not merging will leave an empty line containing the
     542        // end of the selection to delete.
     543        bool mergeBlocksAfterDelete = !fragment.hasInterchangeNewlineAtEnd() && !fragment.hasInterchangeNewlineAtStart() && isEndOfParagraph(visibleEnd);
    538544        deleteSelection(false, mergeBlocksAfterDelete);
    539545        updateLayout();
    … …  
    667673        VisiblePosition visibleInsertionPos(insertionPos);
    668674        fragment.removeNode(refNode);
    669         if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isStartOfBlock(visibleInsertionPos))
     675        // FIXME: The first two cases need to be rethought.  They're about preventing the nesting of
     676        // incoming blocks in the block where the paste is being performed.  But, avoiding nesting doesn't
     677        // always produce the desired visual result, and the decisions are based on isBlockFlow, which
     678        // we're getting rid of.
     679        if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isStartOfBlock(visibleInsertionPos) && !m_lastNodeInserted)
    670680            insertNodeBeforeAndUpdateNodesInserted(refNode.get(), insertionBlock);
    671         else if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isEndOfBlock(visibleInsertionPos)) {
     681        else if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isEndOfBlock(visibleInsertionPos))
    672682            insertNodeAfterAndUpdateNodesInserted(refNode.get(), insertionBlock);
    673         } else if (m_lastNodeInserted && !fragment.isBlockFlow(refNode.get())) {
     683        else if (m_lastNodeInserted && !fragment.isBlockFlow(refNode.get())) {
    674684            // A non-null m_lastNodeInserted means we've done merging above.  That means everything in the first paragraph
    675685            // of the fragment has been merged with everything up to the start of the paragraph where the paste was performed. 
Note: See TracChangeset for help on using the changeset viewer.