Reviewed by Darin Adler.
<rdar://problem/
5549929> CrashTracer: [USER] 35 crashes at WebCore::CharacterData::insertData
We were trying to insert a tab into a br, after the br incorrectly ended up inside
a tab span.
* editing/DeleteButtonController.cpp:
(WebCore::isDeletableElement): Changed to take in a const Node* instead of a Node*.
* editing/DeleteSelectionCommand.cpp:
(WebCore::isTableRow): Ditto.
* editing/IndentOutdentCommand.cpp:
(WebCore::isIndentBlockquote): Ditto.
(WebCore::isListOrIndentBlockquote): Ditto.
* editing/InsertLineBreakCommand.cpp:
(WebCore::InsertLineBreakCommand::shouldUseBreakElement): Added, moved code from
doApply here.
(WebCore::InsertLineBreakCommand::doApply):
Don't upstream() the insertion position. upstream()ing it will only have an effect
when the insertion position is the first in its paragraph (since we canonicalize
VisiblePositions to the upstream() candidate). In this start of paragraph case,
upstream() can move outside inline elements like tab spans or elements that might
have a different whitespace mode (added two test cases to cover these).
Moved code to decide whether to insert a br or a '\n' to its own method.
Removed special case code for inserting at a position inside a tab span. We instead
adjust the insertion position before insertion if it is inside a tab span and
handle insertion in the appropriate if-block. This fixes a bug where we would
only insert one line break when two were needed (added a testcase).
Removed special case code for inserting before and after tables and horizontal
rules. We handle these insertions in the appropriate if-block.
* editing/InsertLineBreakCommand.h:
* editing/ReplaceSelectionCommand.cpp:
(WebCore::isMailPasteAsQuotationNode): Change to take in a const Node*.
* editing/htmlediting.cpp:
(WebCore::isContentEditable): Ditto.
(WebCore::isBlock): Ditto.
(WebCore::enclosingNodeOfType): Changed to take a function pointer to a function
that takes in a const Node*.
(WebCore::isTabSpanTextNode): Check to see that the node actually a text node,
and not, say, a br.
* editing/htmlediting.h:
* editing/markup.cpp:
(WebCore::styleFromMatchedRulesAndInlineDecl): Changed to take in a const Node*.
(WebCore::elementHasTextDecorationProperty): Ditto.
LayoutTests:
Reviewed by Darin Adler.
<rdar://problem/
5549929> CrashTracer: [USER] 35 crashes at WebCore::CharacterData::insertData
* editing/inserting/
5549929-1-expected.txt: Added.
* editing/inserting/
5549929-1.html: Added.
* editing/inserting/
5549929-2.html: Added.
* editing/inserting/
5549929-3.html: Added.
* platform/mac/editing/inserting/
5549929-2-expected.checksum: Added.
* platform/mac/editing/inserting/
5549929-2-expected.png: Added.
* platform/mac/editing/inserting/
5549929-2-expected.txt: Added.
* platform/mac/editing/inserting/
5549929-3-expected.checksum: Added.
* platform/mac/editing/inserting/
5549929-3-expected.png: Added.
* platform/mac/editing/inserting/
5549929-3-expected.txt: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27280
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-10-30 Justin Garcia <set EMAIL_ADDRESS environment variable>
+
+ Reviewed by Darin Adler.
+
+ <rdar://problem/5549929> CrashTracer: [USER] 35 crashes at WebCore::CharacterData::insertData
+
+ * editing/inserting/5549929-1-expected.txt: Added.
+ * editing/inserting/5549929-1.html: Added.
+ * editing/inserting/5549929-2.html: Added.
+ * editing/inserting/5549929-3.html: Added.
+ * platform/mac/editing/inserting/5549929-2-expected.checksum: Added.
+ * platform/mac/editing/inserting/5549929-2-expected.png: Added.
+ * platform/mac/editing/inserting/5549929-2-expected.txt: Added.
+ * platform/mac/editing/inserting/5549929-3-expected.checksum: Added.
+ * platform/mac/editing/inserting/5549929-3-expected.png: Added.
+ * platform/mac/editing/inserting/5549929-3-expected.txt: Added.
+
2007-10-30 Adam Roben <aroben@apple.com>
Add another failing test to the Windows Skipped file
--- /dev/null
+This tests for a crash when inserting into a tab into a tab span that contains a br. You should no longer be able to get a br inside a tab span while editing but we should still avoid the crash.
+
+foo
+
+bar
--- /dev/null
+<p>This tests for a crash when inserting into a tab into a tab span that contains a br. You should no longer be able to get a br inside a tab span while editing but we should still avoid the crash.</p>
+<div id="div" contenteditable="true">
+<div>foo</div>
+<div><span id="span" class="Apple-tab-span"><br></span></div>
+<div>bar</div>
+
+<script>
+if (window.layoutTestController)
+ window.layoutTestController.dumpAsText();
+document.getElementById("div").focus();
+span = document.getElementById("span");
+window.getSelection().setPosition(span, 0);
+document.execCommand("InsertText", false, "\t");
+</script>
--- /dev/null
+<p>This tests to make sure that a br isn't inserted into a tab span during an InsertLineBreak operation. You can test for its existence with the DOM inspector or you can look at the render tree.</p>
+<div id="div" contenteditable="true">
+<div><span class="Apple-tab-span" style="white-space:pre"> </span>foo</div>
+<div><span class="Apple-tab-span" style="white-space:pre"> </span>bar</div>
+</div>
+
+<script>
+div = document.getElementById("div");
+div.focus();
+sel = window.getSelection();
+sel.setPosition(div, 0);
+sel.modify("extend", "forward", "paragraph");
+document.execCommand("InsertLineBreak");
+</script>
--- /dev/null
+<p>This tests inserting a line break at the end of a tab span. Below you should see 'foo' followed by an empty paragraph, with the caret in it.</p>
+<div id="div" contenteditable="true">foo<span class="Apple-tab-span" style="white-space:pre"> </span></div>
+
+<script>
+div = document.getElementById("div");
+sel = window.getSelection();
+sel.setPosition(div, div.childNodes.length);
+document.execCommand("InsertLineBreak");
+</script>
--- /dev/null
+bc90ab348ab49c2cc7edaea67eae9f46
\ No newline at end of file
--- /dev/null
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderBlock {P} at (0,0) size 784x36
+ RenderText {#text} at (0,0) size 735x36
+ text run at (0,0) width 616: "This tests to make sure that a br isn't inserted into a tab span during an InsertLineBreak operation. "
+ text run at (616,0) width 119: "You can test for its"
+ text run at (0,18) width 432: "existence with the DOM inspector or you can look at the render tree."
+ RenderBlock {DIV} at (0,52) size 784x36
+ RenderBlock {DIV} at (0,0) size 784x36
+ RenderBR {BR} at (0,0) size 0x18
+ RenderInline {SPAN} at (0,0) size 32x18
+ RenderText {#text} at (0,18) size 32x18
+ text run at (0,18) width 32: "\x{9}"
+ RenderText {#text} at (32,18) size 20x18
+ text run at (32,18) width 20: "bar"
+caret: position 0 of child 0 {#text} of child 1 {SPAN} of child 1 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
--- /dev/null
+ee0d37eefc69b4205655bd3818ab7277
\ No newline at end of file
--- /dev/null
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderBlock {P} at (0,0) size 784x36
+ RenderText {#text} at (0,0) size 767x36
+ text run at (0,0) width 351: "This tests inserting a line break at the end of a tab span. "
+ text run at (351,0) width 416: "Below you should see 'foo' followed by an empty paragraph, with"
+ text run at (0,18) width 85: "the caret in it."
+ RenderBlock {DIV} at (0,52) size 784x36
+ RenderText {#text} at (0,0) size 21x18
+ text run at (0,0) width 21: "foo"
+ RenderInline {SPAN} at (0,0) size 11x18
+ RenderText {#text} at (21,0) size 11x18
+ text run at (21,0) width 11: "\x{9}"
+ RenderBR {BR} at (32,14) size 0x0
+ RenderBR {BR} at (0,18) size 0x18
+caret: position 0 of child 3 {BR} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+2007-10-30 Justin Garcia <justin.garcia@apple.com>
+
+ Reviewed by Darin Adler.
+
+ <rdar://problem/5549929> CrashTracer: [USER] 35 crashes at WebCore::CharacterData::insertData
+
+ We were trying to insert a tab into a br, after the br incorrectly ended up inside
+ a tab span.
+
+ * editing/DeleteButtonController.cpp:
+ (WebCore::isDeletableElement): Changed to take in a const Node* instead of a Node*.
+ * editing/DeleteSelectionCommand.cpp:
+ (WebCore::isTableRow): Ditto.
+ * editing/IndentOutdentCommand.cpp:
+ (WebCore::isIndentBlockquote): Ditto.
+ (WebCore::isListOrIndentBlockquote): Ditto.
+ * editing/InsertLineBreakCommand.cpp:
+ (WebCore::InsertLineBreakCommand::shouldUseBreakElement): Added, moved code from
+ doApply here.
+ (WebCore::InsertLineBreakCommand::doApply):
+ Don't upstream() the insertion position. upstream()ing it will only have an effect
+ when the insertion position is the first in its paragraph (since we canonicalize
+ VisiblePositions to the upstream() candidate). In this start of paragraph case,
+ upstream() can move outside inline elements like tab spans or elements that might
+ have a different whitespace mode (added two test cases to cover these).
+ Moved code to decide whether to insert a br or a '\n' to its own method.
+ Removed special case code for inserting at a position inside a tab span. We instead
+ adjust the insertion position before insertion if it is inside a tab span and
+ handle insertion in the appropriate if-block. This fixes a bug where we would
+ only insert one line break when two were needed (added a testcase).
+ Removed special case code for inserting before and after tables and horizontal
+ rules. We handle these insertions in the appropriate if-block.
+ * editing/InsertLineBreakCommand.h:
+ * editing/ReplaceSelectionCommand.cpp:
+ (WebCore::isMailPasteAsQuotationNode): Change to take in a const Node*.
+ * editing/htmlediting.cpp:
+ (WebCore::isContentEditable): Ditto.
+ (WebCore::isBlock): Ditto.
+ (WebCore::enclosingNodeOfType): Changed to take a function pointer to a function
+ that takes in a const Node*.
+ (WebCore::isTabSpanTextNode): Check to see that the node actually a text node,
+ and not, say, a br.
+ * editing/htmlediting.h:
+ * editing/markup.cpp:
+ (WebCore::styleFromMatchedRulesAndInlineDecl): Changed to take in a const Node*.
+ (WebCore::elementHasTextDecorationProperty): Ditto.
+
2007-10-30 Antti Koivisto <antti@apple.com>
Reviewed by NOBODY.
{
}
-static bool isDeletableElement(Node* node)
+static bool isDeletableElement(const Node* node)
{
if (!node || !node->isHTMLElement() || !node->inDocument() || !node->isContentEditable())
return false;
return node && (node->hasTagName(tdTag) || node->hasTagName(thTag));
}
-static bool isTableRow(Node* node)
+static bool isTableRow(const Node* node)
{
return node && node->hasTagName(trTag);
}
return indentBlockquoteElement.release();
}
-static bool isIndentBlockquote(Node* node)
+static bool isIndentBlockquote(const Node* node)
{
if (!node || !node->hasTagName(blockquoteTag) || !node->isElementNode())
return false;
- Element* elem = static_cast<Element*>(node);
+ Element* elem = static_cast<const Element*>(node);
return elem->getAttribute(classAttr) == indentBlockquoteString();
}
-static bool isListOrIndentBlockquote(Node* node)
+static bool isListOrIndentBlockquote(const Node* node)
{
return node && (node->hasTagName(ulTag) || node->hasTagName(olTag) || isIndentBlockquote(node));
}
insertNodeBefore(node, pos.node());
}
+// Whether we should insert a break element or a '\n'.
+bool InsertLineBreakCommand::shouldUseBreakElement(const Position& insertionPos)
+{
+ return insertionPos.node()->renderer() && !insertionPos.node()->renderer()->style()->preserveNewline();
+}
+
void InsertLineBreakCommand::doApply()
{
deleteSelection();
return;
VisiblePosition caret(selection.visibleStart());
-
- Position pos(caret.deepEquivalent().upstream());
- Position canonicalPos(caret.deepEquivalent());
+ Position pos(caret.deepEquivalent());
pos = positionAvoidingSpecialElementBoundary(pos);
-
- Node* styleNode = pos.node();
- bool isTabSpan = isTabSpanTextNode(styleNode);
- if (isTabSpan)
- styleNode = styleNode->parentNode()->parentNode();
- RenderObject* styleRenderer = styleNode->renderer();
- bool useBreakElement = !styleRenderer || !styleRenderer->style()->preserveNewline();
+
+ pos = positionOutsideTabSpan(pos);
RefPtr<Node> nodeToInsert;
- if (useBreakElement)
+ if (shouldUseBreakElement(pos))
nodeToInsert = createBreakElement(document());
else
nodeToInsert = document()->createTextNode("\n");
- // FIXME: Need to merge text nodes when inserting just after or before text.
- if (isTabSpan) {
- insertNodeAtTabSpanPosition(nodeToInsert.get(), pos);
- setEndingSelection(Selection(Position(nodeToInsert->traverseNextNode(), 0), DOWNSTREAM));
- } else if (canonicalPos.node()->renderer() && canonicalPos.node()->renderer()->isTable() ||
- canonicalPos.node()->hasTagName(hrTag)) {
- if (canonicalPos.offset() == 0) {
- insertNodeBefore(nodeToInsert.get(), canonicalPos.node());
- // Insert an extra br or '\n' if the just inserted one collapsed.
- if (!isStartOfParagraph(VisiblePosition(Position(nodeToInsert.get(), 0))))
- insertNodeBefore(nodeToInsert->cloneNode(false).get(), nodeToInsert.get());
- // Leave the selection where it was, just before the table/horizontal rule.
- } else if (canonicalPos.offset() == maxDeepOffset(canonicalPos.node())) {
- insertNodeAfter(nodeToInsert.get(), canonicalPos.node());
- setEndingSelection(Selection(VisiblePosition(Position(nodeToInsert.get(), 0))));
- } else
- // There aren't any VisiblePositions like this yet.
- ASSERT_NOT_REACHED();
- } else if (isEndOfParagraph(caret) && !lineBreakExistsAtPosition(caret)) {
+ // FIXME: Need to merge text nodes when inserting just after or before text.
+
+ if (isEndOfParagraph(caret) && !lineBreakExistsAtPosition(caret)) {
+ bool needExtraLineBreak = !pos.node()->hasTagName(hrTag) && !pos.node()->hasTagName(tableTag);
+
insertNodeAt(nodeToInsert.get(), pos);
- insertNodeBefore(nodeToInsert->cloneNode(false).get(), nodeToInsert.get());
+
+ if (needExtraLineBreak)
+ insertNodeBefore(nodeToInsert->cloneNode(false).get(), nodeToInsert.get());
+
VisiblePosition endingPosition(Position(nodeToInsert.get(), 0));
setEndingSelection(Selection(endingPosition));
} else if (pos.offset() <= pos.node()->caretMinOffset()) {
- // Insert node before downstream position, and place caret there as well.
- Position endingPosition = pos.downstream();
- insertNodeBeforePosition(nodeToInsert.get(), endingPosition);
- setEndingSelection(Selection(endingPosition, DOWNSTREAM));
+ insertNodeAt(nodeToInsert.get(), pos);
+
+ // Insert an extra br or '\n' if the just inserted one collapsed.
+ if (!isStartOfParagraph(VisiblePosition(Position(nodeToInsert.get(), 0))))
+ insertNodeBefore(nodeToInsert->cloneNode(false).get(), nodeToInsert.get());
+
+ setEndingSelection(Selection(positionAfterNode(nodeToInsert.get()), DOWNSTREAM));
} else if (pos.offset() >= pos.node()->caretMaxOffset()) {
- // Insert BR after this node. Place caret in the position that is downstream
- // of the current position, reckoned before inserting the BR in between.
- Position endingPosition = pos.downstream();
- insertNodeAfterPosition(nodeToInsert.get(), pos);
- setEndingSelection(Selection(endingPosition, DOWNSTREAM));
+ insertNodeAt(nodeToInsert.get(), pos);
+ setEndingSelection(Selection(positionAfterNode(nodeToInsert.get()), DOWNSTREAM));
} else {
// Split a text node
ASSERT(pos.node()->isTextNode());
virtual bool preservesTypingStyle() const;
void insertNodeAfterPosition(Node*, const Position&);
void insertNodeBeforePosition(Node*, const Position&);
+ bool shouldUseBreakElement(const Position&);
};
} // namespace WebCore
shouldMerge(endOfInsertedContent, next);
}
-static bool isMailPasteAsQuotationNode(Node* node)
+static bool isMailPasteAsQuotationNode(const Node* node)
{
- return node && node->hasTagName(blockquoteTag) && node->isElementNode() && static_cast<Element*>(node)->getAttribute(classAttr) == ApplePasteAsQuotation;
+ return node && node->hasTagName(blockquoteTag) && node->isElementNode() && static_cast<const Element*>(node)->getAttribute(classAttr) == ApplePasteAsQuotation;
}
// Wrap CompositeEditCommand::removeNodePreservingChildren() so we can update the nodes we track
return node->rootEditableElement();
}
-bool isContentEditable(Node* node)
+bool isContentEditable(const Node* node)
{
return node->isContentEditable();
}
}
// Whether or not content before and after this node will collapse onto the same line as it.
-bool isBlock(Node* node)
+bool isBlock(const Node* node)
{
return node && node->renderer() && !node->renderer()->isInline();
}
return 0;
}
-Node* enclosingNodeOfType(Node* node, bool (*nodeIsOfType)(Node*))
+Node* enclosingNodeOfType(Node* node, bool (*nodeIsOfType)(const Node*))
{
if (!node)
return 0;
bool isTabSpanTextNode(const Node *node)
{
- return (node && node->parentNode() && isTabSpanNode(node->parentNode()));
+ return node && node->isTextNode() && node->parentNode() && isTabSpanNode(node->parentNode());
}
Node *tabSpanNode(const Node *node)
VisiblePosition lastEditablePositionBeforePositionInRoot(const Position&, Node*);
int comparePositions(const Position&, const Position&);
Node* lowestEditableAncestor(Node*);
-bool isContentEditable(Node*);
+bool isContentEditable(const Node*);
Position nextCandidate(const Position&);
Position nextVisuallyDistinctCandidate(const Position&);
Position previousCandidate(const Position&);
bool isEditablePosition(const Position&);
bool isRichlyEditablePosition(const Position&);
Element* editableRootForPosition(const Position&);
-bool isBlock(Node*);
+bool isBlock(const Node*);
Node* enclosingBlock(Node*);
String stringWithRebalancedWhitespace(const String&, bool, bool);
Node* isFirstPositionAfterTable(const VisiblePosition&);
Node* enclosingNodeWithTag(Node*, const QualifiedName&);
-Node* enclosingNodeOfType(Node*, bool (*nodeIsOfType)(Node*));
+Node* enclosingNodeOfType(Node*, bool (*nodeIsOfType)(const Node*));
Node* enclosingTableCell(const Position&);
Node* enclosingEmptyListItem(const VisiblePosition&);
Node* enclosingAnchorElement(const Position&);
return isEndOfParagraph(v) && isStartOfParagraph(next) && !(upstreamNode->hasTagName(brTag) && upstreamNode == downstreamNode);
}
-static PassRefPtr<CSSMutableStyleDeclaration> styleFromMatchedRulesAndInlineDecl(Node* node)
+static PassRefPtr<CSSMutableStyleDeclaration> styleFromMatchedRulesAndInlineDecl(const Node* node)
{
if (!node->isHTMLElement())
return 0;
- HTMLElement* element = static_cast<HTMLElement*>(node);
+ HTMLElement* element = static_cast<const HTMLElement*>(node);
RefPtr<CSSMutableStyleDeclaration> style = styleFromMatchedRulesForElement(element);
RefPtr<CSSMutableStyleDeclaration> inlineStyleDecl = element->getInlineStyleDecl();
style->merge(inlineStyleDecl.get());
return static_cast<CSSPrimitiveValue*>(value.get())->getIdent() == CSS_VAL_NONE;
}
-static bool elementHasTextDecorationProperty(Node* node)
+static bool elementHasTextDecorationProperty(const Node* node)
{
RefPtr<CSSMutableStyleDeclaration> style = styleFromMatchedRulesAndInlineDecl(node);
if (!style)