Commit 8073bff2 authored by rniwa@webkit.org's avatar rniwa@webkit.org

2011-04-08 Ryosuke Niwa <rniwa@webkit.org>

        Reviewed by Tony Chang, Darin Adler, and Enrica Casucci.

        REGRESSION(r81887): Crash in SplitElement
        https://bugs.webkit.org/show_bug.cgi?id=57743

        The crash was caused by ReplaceSelectionCommand::doApply's calling splitElement with computeNodeAfterPosition
        even when the position was after the last node in it container. Since all we are doing here is to splitting tree
        up until the highest ancestor with isInlineNodeWithStyle, replaced the while loop by calls to splitTreeToNode
        and highestEnclosingNodeOfType.

        Also fixed a bug in splitTreeToNode not to check the difference in visible position when splitting the ancestor,
        which would have introduced unnecessary nodes when splitting tree and a bug in highestEnclosingNodeOfType that
        it incorrectly called deprecatedNode instead of containerNode.

        Test: editing/inserting/insert-images-in-pre-x-crash.html

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::splitTreeToNode):
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::doApply):
        * editing/htmlediting.cpp:
        (WebCore::highestEnclosingNodeOfType):
2011-04-08  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Tony Chang, Darin Adler, and Enrica Casucci.

        REGRESSION(r81887): Crash in SplitElement
        https://bugs.webkit.org/show_bug.cgi?id=57743

        Added a regression test for a crash in ReplaceSelectionCommand.

        * editing/inserting/insert-images-in-pre-x-crash-expected.txt: Added.
        * editing/inserting/insert-images-in-pre-x-crash.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@83322 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 559a988d
2011-04-08 Ryosuke Niwa <rniwa@webkit.org>
Reviewed by Tony Chang, Darin Adler, and Enrica Casucci.
REGRESSION(r81887): Crash in SplitElement
https://bugs.webkit.org/show_bug.cgi?id=57743
Added a regression test for a crash in ReplaceSelectionCommand.
* editing/inserting/insert-images-in-pre-x-crash-expected.txt: Added.
* editing/inserting/insert-images-in-pre-x-crash.html: Added.
2011-04-08 Antti Koivisto <antti@apple.com>
Reviewed by Tony Gentilcore.
<pre id="x"><x style="white-space: pre-wrap;"><br></x></pre>
<script>
// Adding DOCTYPE, html, or body prevents the crash to reproduce.
if (window.layoutTestController)
layoutTestController.dumpAsText();
var x = document.getElementById("x");
document.execCommand("selectall",false);
document.designMode="on";
document.execCommand("InsertImage");
document.execCommand("InsertImage");
document.execCommand("InsertImage");
document.open();
document.writeln('This test ensures WebKit does not crash.<br><br>PASS');
</script>
</body>
</html>
2011-04-08 Ryosuke Niwa <rniwa@webkit.org>
Reviewed by Tony Chang, Darin Adler, and Enrica Casucci.
REGRESSION(r81887): Crash in SplitElement
https://bugs.webkit.org/show_bug.cgi?id=57743
The crash was caused by ReplaceSelectionCommand::doApply's calling splitElement with computeNodeAfterPosition
even when the position was after the last node in it container. Since all we are doing here is to splitting tree
up until the highest ancestor with isInlineNodeWithStyle, replaced the while loop by calls to splitTreeToNode
and highestEnclosingNodeOfType.
Also fixed a bug in splitTreeToNode not to check the difference in visible position when splitting the ancestor,
which would have introduced unnecessary nodes when splitting tree and a bug in highestEnclosingNodeOfType that
it incorrectly called deprecatedNode instead of containerNode.
Test: editing/inserting/insert-images-in-pre-x-crash.html
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::splitTreeToNode):
* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplaceSelectionCommand::doApply):
* editing/htmlediting.cpp:
(WebCore::highestEnclosingNodeOfType):
2011-04-08 Antti Koivisto <antti@apple.com>
Reviewed by Tony Gentilcore.
......@@ -1217,23 +1217,27 @@ Position CompositeEditCommand::positionAvoidingSpecialElementBoundary(const Posi
// Splits the tree parent by parent until we reach the specified ancestor. We use VisiblePositions
// to determine if the split is necessary. Returns the last split node.
PassRefPtr<Node> CompositeEditCommand::splitTreeToNode(Node* start, Node* end, bool splitAncestor)
PassRefPtr<Node> CompositeEditCommand::splitTreeToNode(Node* start, Node* end, bool shouldSplitAncestor)
{
ASSERT(start);
ASSERT(end);
ASSERT(start != end);
RefPtr<Node> node;
for (node = start; node && node->parentNode() != end; node = node->parentNode()) {
if (shouldSplitAncestor && end->parentNode())
end = end->parentNode();
RefPtr<Node> endNode = end;
for (node = start; node && node->parentNode() != endNode; node = node->parentNode()) {
if (!node->parentNode()->isElementNode())
break;
VisiblePosition positionInParent(firstPositionInNode(node->parentNode()), DOWNSTREAM);
VisiblePosition positionInNode(firstPositionInOrBeforeNode(node.get()), DOWNSTREAM);
// Do not split a node when doing so introduces an empty node.
VisiblePosition positionInParent = firstPositionInNode(node->parentNode());
VisiblePosition positionInNode = firstPositionInOrBeforeNode(node.get());
if (positionInParent != positionInNode)
applyCommandToComposite(SplitElementCommand::create(static_cast<Element*>(node->parentNode()), node));
}
if (splitAncestor) {
splitElement(static_cast<Element*>(end), node);
return node->parentNode();
splitElement(static_cast<Element*>(node->parentNode()), node);
}
return node.release();
}
......
......@@ -952,20 +952,17 @@ void ReplaceSelectionCommand::doApply()
// We can skip this optimization for fragments not wrapped in one of
// our style spans and for positions inside list items
// since insertAsListItems already does the right thing.
if (!m_matchStyle && !enclosingList(insertionPos.anchorNode()) && isStyleSpan(fragment.firstChild())) {
Node* parentNode = insertionPos.anchorNode()->parentNode();
while (parentNode && parentNode->renderer() && isInlineNodeWithStyle(parentNode)) {
// If we are in the middle of a text node, we need to split it before we can
// move the insertion position.
if (insertionPos.anchorNode()->isTextNode() && insertionPos.anchorType() == Position::PositionIsOffsetInAnchor && insertionPos.offsetInContainerNode() && !insertionPos.atLastEditingPositionForNode())
splitTextNodeContainingElement(static_cast<Text*>(insertionPos.anchorNode()), insertionPos.offsetInContainerNode());
// If the style element has more than one child, we need to split it.
if (parentNode->firstChild()->nextSibling())
splitElement(static_cast<Element*>(parentNode), insertionPos.computeNodeAfterPosition());
insertionPos = positionInParentBeforeNode(parentNode);
parentNode = parentNode->parentNode();
if (!m_matchStyle && !enclosingList(insertionPos.containerNode()) && isStyleSpan(fragment.firstChild())) {
if (insertionPos.containerNode()->isTextNode() && insertionPos.offsetInContainerNode() && !insertionPos.atLastEditingPositionForNode()) {
splitTextNodeContainingElement(static_cast<Text*>(insertionPos.containerNode()), insertionPos.offsetInContainerNode());
insertionPos = firstPositionInNode(insertionPos.containerNode());
}
// FIXME: isInlineNodeWithStyle does not check editability.
if (RefPtr<Node> nodeToSplitTo = highestEnclosingNodeOfType(insertionPos, isInlineNodeWithStyle)) {
if (insertionPos.containerNode() != nodeToSplitTo)
nodeToSplitTo = splitTreeToNode(insertionPos.anchorNode(), nodeToSplitTo.get(), true).get();
insertionPos = positionInParentBeforeNode(nodeToSplitTo.get());
}
}
......
......@@ -638,7 +638,7 @@ Node* highestEnclosingNodeOfType(const Position& p, bool (*nodeIsOfType)(const N
{
Node* highest = 0;
Node* root = rule == CannotCrossEditingBoundary ? highestEditableRoot(p) : 0;
for (Node* n = p.deprecatedNode(); n; n = n->parentNode()) {
for (Node* n = p.containerNode(); n; n = n->parentNode()) {
if (root && !n->rendererIsEditable())
continue;
if (nodeIsOfType(n))
......
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