Changeset 31161 in webkit


Ignore:
Timestamp:
Mar 19, 2008, 2:57:10 PM (17 years ago)
Author:
justin.garcia@apple.com
Message:

WebCore:

2008-03-19 Justin Garcia <​justin.garcia@apple.com>

Reviewed by Oliver.

<rdar://problem/5794920> Acid3: Assertion failure in VisiblePosition::previous when clicking on results (17004)


The position inside an empty inline-block was a candidate, but upstream and downstream
would move across it without stopping. This confused canonicalPosition, since no more
than two candidates should have the same upstream/downstream (be visually equivalent).


Code was added intentionally in isCandidate to make VisiblePositions inside empty
inline-blocks, so we need to make upstream/downstream understand that.

  • dom/Position.cpp: (WebCore::endsOfNodeAreVisuallyDistinctPositions): upstream and downstream used to only stop when entering or leaving a non-inline element (referred to as a "block"). We must also avoid entering or leaving an empty inline-block. This will allow a VisiblePosition there, to match up with what the code in isCandidate intended. (WebCore::enclosingVisualBoundary): Removed enclosingBlock and replaced it with this. (WebCore::Position::upstream): Added better comments, called the new functions. (WebCore::Position::downstream): Ditto.
  • dom/Position.h:

LayoutTests:

2008-03-19 Justin Garcia <​justin.garcia@apple.com>

Reviewed by Oliver.


<rdar://problem/5794920> Acid3: Assertion failure in VisiblePosition::previous when clicking on results (17004)

  • editing/pasteboard/4989774.html: Updated to wait for the images to load before trying to copy it.
  • editing/selection/5794920-1-expected.txt: Added.
  • editing/selection/5794920-1.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r31157 r31161  
     12008-03-19  Justin Garcia  <justin.garcia@apple.com>
     2
     3        Reviewed by Oliver.
     4       
     5        <rdar://problem/5794920> Acid3: Assertion failure in VisiblePosition::previous when clicking on results (17004)
     6
     7        * editing/pasteboard/4989774.html: Updated to wait for the images to load before trying to copy it.
     8        * editing/selection/5794920-1-expected.txt: Added.
     9        * editing/selection/5794920-1.html: Added.
     10
    1112008-03-19  Dan Bernstein  <mitz@apple.com>
    212

  • trunk/LayoutTests/editing/pasteboard/4989774.html

    r20963 r31161  
    22
    33<script>
     4function partTwo() {
     5    sel.setBaseAndExtent(body, 0, body, body.childNodes.length);
     6    document.execCommand("Copy");
     7    sel.setPosition(body, body.childNodes.length);
     8    document.execCommand("Paste");
     9    document.execCommand("Paste");
     10    document.execCommand("InsertLineBreak");
     11    document.execCommand("InsertText", false, "This tests for a bug where an images pasted on the same line would appear on different lines.  You should see several pictures above all in the same line/paragraph.");
     12    window.layoutTestController.notifyDone();   
     13}
     14
    415var body = document.body;
    516var sel = window.getSelection();
    6 sel.setBaseAndExtent(body, 0, body, body.childNodes.length);
    7 
    8 document.execCommand("Copy");
    9 sel.setPosition(body, body.childNodes.length);
    10 document.execCommand("Paste");
    11 document.execCommand("Paste");
    12 document.execCommand("InsertLineBreak");
    13 document.execCommand("InsertText", false, "This tests for a bug where an images pasted on the same line would appear on different lines.  You should see several pictures above all in the same line/paragraph.");
     17if (window.layoutTestController) {
     18    window.layoutTestController.waitUntilDone();
     19   
     20    window.setTimeout(partTwo, 200);
     21} else {
     22    document.execCommand("SelectAll")
     23    document.execCommand("InsertText", false, "This tests for a bug where an images pasted on the same line would appear on different lines. To run it manually, Undo, then select the image, Copy, put the caret after the image, then Paste twice. All three images should be in separate paragraphs.");   
     24}
    1425</script>

  • trunk/WebCore/ChangeLog

    r31160 r31161  
     12008-03-19  Justin Garcia  <justin.garcia@apple.com>
     2
     3        Reviewed by Oliver.
     4
     5        <rdar://problem/5794920> Acid3: Assertion failure in VisiblePosition::previous when clicking on results (17004)
     6       
     7        The position inside an empty inline-block was a candidate, but upstream and downstream
     8        would move across it without stopping.  This confused canonicalPosition, since no more
     9        than two candidates should have the same upstream/downstream (be visually equivalent).
     10       
     11        Code was added intentionally in isCandidate to make VisiblePositions inside empty
     12        inline-blocks, so we need to make upstream/downstream understand that.
     13
     14        * dom/Position.cpp:
     15        (WebCore::endsOfNodeAreVisuallyDistinctPositions): upstream and downstream used to only
     16        stop when entering or leaving a non-inline element (referred to as a "block").  We must also
     17        avoid entering or leaving an empty inline-block.  This will allow a VisiblePosition there, to
     18        match up with what the code in isCandidate intended.
     19        (WebCore::enclosingVisualBoundary): Removed enclosingBlock and replaced it with this.
     20        (WebCore::Position::upstream): Added better comments, called the new functions.
     21        (WebCore::Position::downstream): Ditto.
     22        * dom/Position.h:
     23
    1242008-03-19  Dan Bernstein  <mitz@apple.com>
    225

  • trunk/WebCore/dom/Position.cpp

    r30973 r31161  
    3535#include "Logging.h"
    3636#include "PositionIterator.h"
    37 #include "RenderBlock.h"
    3837#include "Text.h"
    3938#include "TextIterator.h"
    … …  
    266265}
    267266
     267// Whether or not [node, 0] and [node, maxDeepOffset(node)] are their own VisiblePositions.
     268// If true, adjacent candidates are visually distinct.
     269// FIXME: Disregard nodes with renderers that have no height, as we do in isCandidate.
     270// FIXME: Share code with isCandidate, if possible.
     271static bool endsOfNodeAreVisuallyDistinctPositions(Node* node)
     272{
     273    if (!node || !node->renderer())
     274        return false;
     275       
     276    if (!node->renderer()->isInline())
     277        return true;
     278       
     279    // Don't include inline tables.
     280    if (node->hasTagName(tableTag))
     281        return false;
     282   
     283    // There is a VisiblePosition inside an empty inline-block container.
     284    return node->renderer()->isReplaced() && canHaveChildrenForEditing(node) && node->renderer()->height() != 0 && !node->firstChild();
     285}
     286
     287static Node* enclosingVisualBoundary(Node* node)
     288{
     289    while (node && !endsOfNodeAreVisuallyDistinctPositions(node))
     290        node = node->parentNode();
     291       
     292    return node;
     293}
     294
    268295// upstream() and downstream() want to return positions that are either in a
    269296// text node or at just before a non-text node.  This method checks for that.
    … …  
    279306}
    280307
    281 // enclosingBlock does some expensive editability checks, upstream and downstream
    282 // can avoid those because they do their own editability checking.
    283 static Node* enclosingBlockIgnoringEditability(Node* node)
    284 {
    285     while (node && !isBlock(node))
    286         node = node->parentNode();
    287        
    288     return node;
    289 }
    290 
    291 // p.upstream() returns the start of the range of positions that map to the same VisiblePosition as P.
     308// This function and downstream() are used for moving back and forth between visually equivalent candidates.
     309// For example, for the text node "foo     bar" where whitespace is collapsible, there are two candidates
     310// that map to the VisiblePosition between 'b' and the space.  This function will return the left candidate
     311// and downstream() will return the right one.
     312// Also, upstream() will return [boundary, 0] for any of the positions from [boundary, 0] to the first candidate
     313// in boundary, where endsOfNodeAreVisuallyDistinctPositions(boundary) is true.
    292314Position Position::upstream() const
    293315{
    … …  
    297319   
    298320    // iterate backward from there, looking for a qualified position
    299     Node* originalBlock = enclosingBlockIgnoringEditability(startNode);
     321    Node* boundary = enclosingVisualBoundary(startNode);
    300322    PositionIterator lastVisible = *this;
    301323    PositionIterator currentPos = lastVisible;
    … …  
    315337        }
    316338
    317         // Don't enter a new enclosing block flow or table element.  There is code below that
    318         // terminates early if we're about to leave a block.
    319         if (isBlock(currentNode) && currentNode != originalBlock)
     339        // If we've moved to a position that is visually disinct, return the last saved position. There
     340        // is code below that terminates early if we're *about* to move to a visually distinct position.
     341        if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentNode != boundary)
    320342            return lastVisible;
    321343
    … …  
    329351            lastVisible = currentPos;
    330352       
    331         // Don't leave a block flow or table element.  We could rely on code above to terminate and
     353        // Don't move past a position that is visually distinct.  We could rely on code above to terminate and
    332354        // return lastVisible on the next iteration, but we terminate early to avoid doing a nodeIndex() call.
    333         if (isBlock(currentNode) && currentPos.atStartOfNode())
     355        if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentPos.atStartOfNode())
    334356            return lastVisible;
    335357
    … …  
    369391}
    370392
    371 // P.downstream() returns the end of the range of positions that map to the same VisiblePosition as P.
     393// This function and upstream() are used for moving back and forth between visually equivalent candidates.
     394// For example, for the text node "foo     bar" where whitespace is collapsible, there are two candidates
     395// that map to the VisiblePosition between 'b' and the space.  This function will return the right candidate
     396// and upstream() will return the left one.
     397// Also, downstream() will return the last position in the last atomic node in boundary for all of the positions
     398// in boundary after the last candidate, where endsOfNodeAreVisuallyDistinctPositions(boundary).
    372399Position Position::downstream() const
    373400{
    … …  
    377404
    378405    // iterate forward from there, looking for a qualified position
    379     Node* originalBlock = enclosingBlockIgnoringEditability(startNode);
     406    Node* boundary = enclosingVisualBoundary(startNode);
    380407    PositionIterator lastVisible = *this;
    381408    PositionIterator currentPos = lastVisible;
    … …  
    400427            break;
    401428           
    402         // Do not enter a new enclosing block.
    403         if (isBlock(currentNode) && currentNode != originalBlock)
     429        // Do not move to a visually distinct position.
     430        if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentNode != boundary)
    404431            return lastVisible;
    405         // Do not leave the original enclosing block.
    406         // Note: The first position after the last one in the original block
    407         // will be [originalBlock->parentNode(), originalBlock->nodeIndex() + 1].
    408         if (originalBlock && originalBlock->parentNode() == currentNode)
     432        // Do not move past a visually disinct position.
     433        // Note: The first position after the last in a node whose ends are visually distinct
     434        // positions will be [boundary->parentNode(), originalBlock->nodeIndex() + 1].
     435        if (boundary && boundary->parentNode() == currentNode)
    409436            return lastVisible;
    410437

  • trunk/WebCore/dom/Position.h

    r30973 r31161  
    8080    Position leadingWhitespacePosition(EAffinity, bool considerNonCollapsibleWhitespace = false) const;
    8181    Position trailingWhitespacePosition(EAffinity, bool considerNonCollapsibleWhitespace = false) const;
    82 
    83     // p.upstream() through p.downstream() is the range of positions that map to the same VisiblePosition as p.
     82   
     83    // These return useful visually equivalent positions.
    8484    Position upstream() const;
    8585    Position downstream() const;
Note: See TracChangeset for help on using the changeset viewer.