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.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@31161 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 920b8e07
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.
2008-03-19 Dan Bernstein <mitz@apple.com>
Reviewed by Sam Weinig.
<body style="font-weight: bold;" id="div" contenteditable="true"><img src="../resources/abe.png"></body>
<script>
function partTwo() {
sel.setBaseAndExtent(body, 0, body, body.childNodes.length);
document.execCommand("Copy");
sel.setPosition(body, body.childNodes.length);
document.execCommand("Paste");
document.execCommand("Paste");
document.execCommand("InsertLineBreak");
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.");
window.layoutTestController.notifyDone();
}
var body = document.body;
var sel = window.getSelection();
sel.setBaseAndExtent(body, 0, body, body.childNodes.length);
document.execCommand("Copy");
sel.setPosition(body, body.childNodes.length);
document.execCommand("Paste");
document.execCommand("Paste");
document.execCommand("InsertLineBreak");
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.");
if (window.layoutTestController) {
window.layoutTestController.waitUntilDone();
window.setTimeout(partTwo, 200);
} else {
document.execCommand("SelectAll")
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.");
}
</script>
This tests for a crash when doing selection operations next to empty inline blocks. You should see HelloWorld below, and the 'W' should be inside the inline-block.
Test Passed (there was no crash)
<div id="description">This tests for a crash when doing selection operations next to empty inline blocks. You should see HelloWorld below, and the 'W' should be inside the inline-block.</div>
<div id="edit" contenteditable="true">Hello<div><div id="div" style="display:inline-block; border: 1px solid red; height: 10px;"></div><div style="display:inline-block; border: 1px solid red; height: 10px;"></div> </div>World</div>
<script>
edit = document.getElementById("edit");
text = edit.childNodes[edit.childNodes.length - 1];
window.getSelection().setPosition(text, 2);
window.getSelection().modify("move", "backward", "character");
if (window.layoutTestController) {
window.layoutTestController.dumpAsText();
document.body.innerText = document.getElementById("description").innerHTML + "\n\nTest Passed (there was no crash)";
}
</script>
\ No newline at end of file
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:
2008-03-19 Dan Bernstein <mitz@apple.com>
Rubber-stamped by John Sullivan.
......@@ -34,7 +34,6 @@
#include "HTMLNames.h"
#include "Logging.h"
#include "PositionIterator.h"
#include "RenderBlock.h"
#include "Text.h"
#include "TextIterator.h"
#include "htmlediting.h"
......@@ -265,6 +264,34 @@ Position Position::nextCharacterPosition(EAffinity affinity) const
return *this;
}
// Whether or not [node, 0] and [node, maxDeepOffset(node)] are their own VisiblePositions.
// If true, adjacent candidates are visually distinct.
// FIXME: Disregard nodes with renderers that have no height, as we do in isCandidate.
// FIXME: Share code with isCandidate, if possible.
static bool endsOfNodeAreVisuallyDistinctPositions(Node* node)
{
if (!node || !node->renderer())
return false;
if (!node->renderer()->isInline())
return true;
// Don't include inline tables.
if (node->hasTagName(tableTag))
return false;
// There is a VisiblePosition inside an empty inline-block container.
return node->renderer()->isReplaced() && canHaveChildrenForEditing(node) && node->renderer()->height() != 0 && !node->firstChild();
}
static Node* enclosingVisualBoundary(Node* node)
{
while (node && !endsOfNodeAreVisuallyDistinctPositions(node))
node = node->parentNode();
return node;
}
// upstream() and downstream() want to return positions that are either in a
// text node or at just before a non-text node. This method checks for that.
static bool isStreamer(const PositionIterator& pos)
......@@ -278,17 +305,12 @@ static bool isStreamer(const PositionIterator& pos)
return pos.atStartOfNode();
}
// enclosingBlock does some expensive editability checks, upstream and downstream
// can avoid those because they do their own editability checking.
static Node* enclosingBlockIgnoringEditability(Node* node)
{
while (node && !isBlock(node))
node = node->parentNode();
return node;
}
// p.upstream() returns the start of the range of positions that map to the same VisiblePosition as P.
// This function and downstream() are used for moving back and forth between visually equivalent candidates.
// For example, for the text node "foo bar" where whitespace is collapsible, there are two candidates
// that map to the VisiblePosition between 'b' and the space. This function will return the left candidate
// and downstream() will return the right one.
// Also, upstream() will return [boundary, 0] for any of the positions from [boundary, 0] to the first candidate
// in boundary, where endsOfNodeAreVisuallyDistinctPositions(boundary) is true.
Position Position::upstream() const
{
Node* startNode = node();
......@@ -296,7 +318,7 @@ Position Position::upstream() const
return Position();
// iterate backward from there, looking for a qualified position
Node* originalBlock = enclosingBlockIgnoringEditability(startNode);
Node* boundary = enclosingVisualBoundary(startNode);
PositionIterator lastVisible = *this;
PositionIterator currentPos = lastVisible;
bool startEditable = startNode->isContentEditable();
......@@ -314,9 +336,9 @@ Position Position::upstream() const
lastNode = currentNode;
}
// Don't enter a new enclosing block flow or table element. There is code below that
// terminates early if we're about to leave a block.
if (isBlock(currentNode) && currentNode != originalBlock)
// If we've moved to a position that is visually disinct, return the last saved position. There
// is code below that terminates early if we're *about* to move to a visually distinct position.
if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentNode != boundary)
return lastVisible;
// skip position in unrendered or invisible node
......@@ -328,9 +350,9 @@ Position Position::upstream() const
if (isStreamer(currentPos))
lastVisible = currentPos;
// Don't leave a block flow or table element. We could rely on code above to terminate and
// Don't move past a position that is visually distinct. We could rely on code above to terminate and
// return lastVisible on the next iteration, but we terminate early to avoid doing a nodeIndex() call.
if (isBlock(currentNode) && currentPos.atStartOfNode())
if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentPos.atStartOfNode())
return lastVisible;
// Return position after tables and nodes which have content that can be ignored.
......@@ -368,7 +390,12 @@ Position Position::upstream() const
return lastVisible;
}
// P.downstream() returns the end of the range of positions that map to the same VisiblePosition as P.
// This function and upstream() are used for moving back and forth between visually equivalent candidates.
// For example, for the text node "foo bar" where whitespace is collapsible, there are two candidates
// that map to the VisiblePosition between 'b' and the space. This function will return the right candidate
// and upstream() will return the left one.
// Also, downstream() will return the last position in the last atomic node in boundary for all of the positions
// in boundary after the last candidate, where endsOfNodeAreVisuallyDistinctPositions(boundary).
Position Position::downstream() const
{
Node* startNode = node();
......@@ -376,7 +403,7 @@ Position Position::downstream() const
return Position();
// iterate forward from there, looking for a qualified position
Node* originalBlock = enclosingBlockIgnoringEditability(startNode);
Node* boundary = enclosingVisualBoundary(startNode);
PositionIterator lastVisible = *this;
PositionIterator currentPos = lastVisible;
bool startEditable = startNode->isContentEditable();
......@@ -399,13 +426,13 @@ Position Position::downstream() const
if (currentNode->hasTagName(bodyTag) && currentPos.atEndOfNode())
break;
// Do not enter a new enclosing block.
if (isBlock(currentNode) && currentNode != originalBlock)
// Do not move to a visually distinct position.
if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentNode != boundary)
return lastVisible;
// Do not leave the original enclosing block.
// Note: The first position after the last one in the original block
// will be [originalBlock->parentNode(), originalBlock->nodeIndex() + 1].
if (originalBlock && originalBlock->parentNode() == currentNode)
// Do not move past a visually disinct position.
// Note: The first position after the last in a node whose ends are visually distinct
// positions will be [boundary->parentNode(), originalBlock->nodeIndex() + 1].
if (boundary && boundary->parentNode() == currentNode)
return lastVisible;
// skip position in unrendered or invisible node
......
......@@ -79,8 +79,8 @@ public:
// These aren't really basic "position" operations. More high level editing helper functions.
Position leadingWhitespacePosition(EAffinity, bool considerNonCollapsibleWhitespace = false) const;
Position trailingWhitespacePosition(EAffinity, bool considerNonCollapsibleWhitespace = false) const;
// p.upstream() through p.downstream() is the range of positions that map to the same VisiblePosition as p.
// These return useful visually equivalent positions.
Position upstream() const;
Position downstream() const;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment