2008-12-23 Darin Adler <darin@apple.com>
Reviewed by John Sullivan.
- improve robustness of undo/redo in HTML editing to fix the following bugs
<https://bugs.webkit.org/show_bug.cgi?id=19703> Crash in WebCore::InsertNodeBeforeCommand::doUnapply()
<rdar://problem/4059423> DOM operations performed on editable HTML can cause a crash later during Undo
Major categories of improvements:
1) Added null checks.
2) Eliminated type casts without corresponding type checks.
3) Avoided possible infinite loops by building up lists of nodes to operate on
before starting to make DOM changes.
4) Use more RefPtr.
No test at this time, but test cases should follow in separate patches.
* WebCore.xcodeproj/project.pbxproj: Set the role of CSSPropertyNames.h to Private so it
can be used in other Private headers, specifically editing ones.
* css/CSSStyleSelector.cpp:
(WebCore::CSSStyleSelector::locateCousinList): Adopt parentElement.
(WebCore::CSSStyleSelector::locateSharedStyle): Ditto.
(WebCore::CSSStyleSelector::SelectorChecker::checkOneSelector): Ditto.
* dom/Element.cpp: (WebCore::Element::cloneElement): Added.
* dom/Element.h: Added cloneElement and an implementation of parentElement.
* dom/Node.h: Moved parentElement from here to Element.h and changed its
implementation so it will return 0 when the parent is not an element
(document, document fragment, etc.).
* editing/AppendNodeCommand.cpp:
(WebCore::AppendNodeCommand::AppendNodeCommand): Made parent be an Element.
Moved assertions from doApply in here.
(WebCore::AppendNodeCommand::doApply): Simplified to just a single unchecked
appendChild call.
(WebCore::AppendNodeCommand::doUnapply): Simplified to just a single remove call.
* editing/AppendNodeCommand.h: Updated.
* editing/ApplyStyleCommand.cpp:
(WebCore::createStyleSpanElement): Eliminate casting by creating an element in a more
direct way with new instead of createElementNS.
(WebCore::ApplyStyleCommand::ApplyStyleCommand): Use PassRefPtr.
(WebCore::ApplyStyleCommand::removeCSSStyle): Use CSSPropertyID.
(WebCore::ApplyStyleCommand::addInlineStyleIfNeeded): Use cloneElement.
* editing/ApplyStyleCommand.h:
* editing/BreakBlockquoteCommand.cpp:
(WebCore::BreakBlockquoteCommand::doApply): Use Element* and cloneElement.
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::applyStyledElement): Use PassRefPtr and unsigned.
(WebCore::CompositeEditCommand::removeStyledElement): Ditto.
(WebCore::CompositeEditCommand::insertNodeBefore): Ditto.
(WebCore::CompositeEditCommand::insertNodeAfter): Ditto.
(WebCore::CompositeEditCommand::insertNodeAt): Ditto.
(WebCore::CompositeEditCommand::appendNode): Ditto.
(WebCore::CompositeEditCommand::removeChildrenInRange): Ditto. Also use a vector to
make the list of children in case removing them has side effects.
(WebCore::CompositeEditCommand::removeNode): Ditto.
(WebCore::CompositeEditCommand::removeNodePreservingChildren): Ditto.
(WebCore::CompositeEditCommand::removeNodeAndPruneAncestors): Ditto.
(WebCore::CompositeEditCommand::splitTextNode): Ditto.
(WebCore::CompositeEditCommand::splitElement): Ditto.
(WebCore::CompositeEditCommand::mergeIdenticalElements): Ditto.
(WebCore::CompositeEditCommand::wrapContentsInDummySpan): Ditto.
(WebCore::CompositeEditCommand::splitTextNodeContainingElement): Ditto.
(WebCore::CompositeEditCommand::joinTextNodes): Ditto.
(WebCore::CompositeEditCommand::inputText): Ditto.
(WebCore::CompositeEditCommand::insertTextIntoNode): Ditto.
(WebCore::CompositeEditCommand::deleteTextFromNode): Ditto.
(WebCore::CompositeEditCommand::replaceTextInNode): Ditto.
(WebCore::CompositeEditCommand::insertNodeAtTabSpanPosition): Ditto.
(WebCore::CompositeEditCommand::removeCSSProperty): Ditto.
(WebCore::CompositeEditCommand::removeNodeAttribute): Ditto. Implement by calling
setNodeAttribute instead of with its own SimpleEditCommand.
(WebCore::CompositeEditCommand::setNodeAttribute): Ditto.
(WebCore::CompositeEditCommand::deleteInsignificantText): Ditto.
(WebCore::CompositeEditCommand::appendBlockPlaceholder): Ditto.
(WebCore::CompositeEditCommand::addBlockPlaceholderIfNeeded): Ditto.
(WebCore::CompositeEditCommand::insertNewDefaultParagraphElementAt): Ditto. Don't
bother using an undoable operation to put the break element into the paragraph
element because there's no need to split them and redo this when doing undo/redo.
(WebCore::CompositeEditCommand::moveParagraphs): Ditto.
(WebCore::CompositeEditCommand::breakOutOfEmptyListItem): Ditto.
* editing/CompositeEditCommand.h: Ditto.
* editing/DeleteFromTextNodeCommand.cpp:
(WebCore::DeleteFromTextNodeCommand::DeleteFromTextNodeCommand): Use unsigned.
(WebCore::DeleteFromTextNodeCommand::doApply): Eliminated inappropriate assertions.
(WebCore::DeleteFromTextNodeCommand::doUnapply): Ditto.
* editing/DeleteFromTextNodeCommand.h:
* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::removeNode): Use PassRefPtr.
(WebCore::DeleteSelectionCommand::deleteTextFromNode): Ditto.
* editing/DeleteSelectionCommand.h:
* editing/FormatBlockCommand.cpp:
(WebCore::FormatBlockCommand::FormatBlockCommand): Use AtomicString.
(WebCore::FormatBlockCommand::doApply): Use Element.
* editing/FormatBlockCommand.h:
* editing/IndentOutdentCommand.cpp:
(WebCore::createIndentBlockquoteElement): Use new to create the element
instead of calling a function so we have a more specific type.
(WebCore::IndentOutdentCommand::prepareBlockquoteLevelForInsertion):
Use RefPtr and Element.
(WebCore::IndentOutdentCommand::indentRegion): Ditto.
(WebCore::IndentOutdentCommand::outdentParagraph): Ditto.
* editing/IndentOutdentCommand.h:
* editing/InsertIntoTextNodeCommand.cpp:
(WebCore::InsertIntoTextNodeCommand::InsertIntoTextNodeCommand):
Use unsigned. Added an assertion.
(WebCore::InsertIntoTextNodeCommand::doApply): Eliminated inappropriate assertions.
(WebCore::InsertIntoTextNodeCommand::doUnapply): Ditto.
* editing/InsertIntoTextNodeCommand.h:
* editing/InsertLineBreakCommand.cpp:
(WebCore::InsertLineBreakCommand::insertNodeAfterPosition): Use Element.
(WebCore::InsertLineBreakCommand::insertNodeBeforePosition): Ditto.
* editing/InsertListCommand.cpp:
(WebCore::InsertListCommand::doApply): Use Element.
* editing/InsertNodeBeforeCommand.cpp:
(WebCore::InsertNodeBeforeCommand::InsertNodeBeforeCommand): Moved assertions
here from doApply.
(WebCore::InsertNodeBeforeCommand::doApply): Eliminated inappropriate assertions.
Added a null check.
(WebCore::InsertNodeBeforeCommand::doUnapply): Simplified to just a single remove call.
* editing/InsertParagraphSeparatorCommand.cpp:
(WebCore::InsertParagraphSeparatorCommand::doApply): Use Element and cloneElement.
* editing/JoinTextNodesCommand.cpp:
(WebCore::JoinTextNodesCommand::doApply): Eliminated inappropriate assertions.
Added some runtime checks. Don't store anything in m_offset.
(WebCore::JoinTextNodesCommand::doUnapply): Ditto.
* editing/JoinTextNodesCommand.h:
* editing/MergeIdenticalElementsCommand.cpp:
(WebCore::MergeIdenticalElementsCommand::MergeIdenticalElementsCommand): Moved
an assertion here from doApply.
(WebCore::MergeIdenticalElementsCommand::doApply): Eliminated inappropriate assertions.
Added a null check. Changed implementation to use remove to avoid null parent issue.
Use a vector of nodes to avoid possible infinite loop if mutation happens while iterating.
(WebCore::MergeIdenticalElementsCommand::doUnapply): Ditto.
* editing/ModifySelectionListLevel.cpp:
(WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange): Use Element*.
(WebCore::IncreaseSelectionListLevelCommand::doApply): Ditto.
* editing/ModifySelectionListLevel.h:
* editing/RemoveCSSPropertyCommand.cpp:
(WebCore::RemoveCSSPropertyCommand::RemoveCSSPropertyCommand): Use PassRefPtr and
CSSPropertyID. Also renamed m_decl to m_style.
(WebCore::RemoveCSSPropertyCommand::doApply): Eliminated inappropriate assertions.
(WebCore::RemoveCSSPropertyCommand::doUnapply): Ditto.
* editing/RemoveNodeAttributeCommand.cpp: Removed contents of this file. To be deleted.
Use SetNodeAttributeCommand instead.
* editing/RemoveNodeAttributeCommand.h: Ditto.
* editing/RemoveNodeCommand.cpp:
(WebCore::RemoveNodeCommand::RemoveNodeCommand): Moved assertions here from doApply.
Don't initialize m_refChild here; rather do it in doApply.
(WebCore::RemoveNodeCommand::doApply): Eliminated inappropriate assertions. Added
checks and streamlined implementation.
(WebCore::RemoveNodeCommand::doUnapply): Ditto.
* editing/RemoveNodeCommand.h:
* editing/RemoveNodePreservingChildrenCommand.cpp:
(WebCore::RemoveNodePreservingChildrenCommand::doApply): Use a vector.
* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplacementFragment::insertFragmentForTestRendering): Removed now-unneeded cast.
* editing/SetNodeAttributeCommand.cpp:
(WebCore::SetNodeAttributeCommand::SetNodeAttributeCommand): Use AtomicString.
Removed assertion that prevents us from using this to remove an attribute.
(WebCore::SetNodeAttributeCommand::doApply): Eliminated inappropriate assertions.
(WebCore::SetNodeAttributeCommand::doUnapply): Ditto.
* editing/SetNodeAttributeCommand.h:
* editing/SplitElementCommand.cpp:
(WebCore::SplitElementCommand::SplitElementCommand): Moved assertion here from doApply.
(WebCore::SplitElementCommand::doApply): Check some more invariants and use a vector
to avoid possible infinite loops.
(WebCore::SplitElementCommand::doUnapply): Ditto.
* editing/SplitTextNodeCommand.cpp:
(WebCore::SplitTextNodeCommand::SplitTextNodeCommand): Moved assertions and comment
here from doApply.
(WebCore::SplitTextNodeCommand::doApply): Check for null and failures when applying.
(WebCore::SplitTextNodeCommand::doUnapply): Ditto.
* editing/SplitTextNodeContainingElementCommand.cpp:
(WebCore::SplitTextNodeContainingElementCommand::doApply): Use Element.
* editing/WrapContentsInDummySpanCommand.cpp:
(WebCore::WrapContentsInDummySpanCommand::doApply): Check for null and ignore failures.
Don't reuse the dummy span. Simplified logic.
(WebCore::WrapContentsInDummySpanCommand::doUnapply): Ditto.
* editing/htmlediting.cpp:
(WebCore::isBlock): Make sure this returns true only for elements.
(WebCore::enclosingBlock): Return an Element*.
(WebCore::enclosingTableCell): Ditto.
(WebCore::enclosingList): Return an HTMLElement*.
(WebCore::outermostEnclosingList): Return an HTMLElement*.
(WebCore::createDefaultParagraphElement): Return an HTMLElement*.
(WebCore::createBreakElement): Return an HTMLElement*.
(WebCore::createOrderedListElement): Return an HTMLElement*.
(WebCore::createUnorderedListElement): Return an HTMLElement*.
(WebCore::createListItemElement): Return an HTMLElement*.
(WebCore::createHTMLElement): Return an HTMLElement*.
* editing/htmlediting.h:
* editing/markup.cpp:
(WebCore::createFragmentFromText): Use createBreakElement and use Element*.
* page/MouseEventWithHitTestResults.cpp:
(WebCore::MouseEventWithHitTestResults::targetNode): Use parentElement.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@39456 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/editing/InsertIntoTextNodeCommand.cpp b/WebCore/editing/InsertIntoTextNodeCommand.cpp
index 9d5ea6e8..4905fae 100644
--- a/WebCore/editing/InsertIntoTextNodeCommand.cpp
+++ b/WebCore/editing/InsertIntoTextNodeCommand.cpp
@@ -30,29 +30,27 @@
namespace WebCore {
-InsertIntoTextNodeCommand::InsertIntoTextNodeCommand(PassRefPtr<Text> node, int offset, const String& text)
+InsertIntoTextNodeCommand::InsertIntoTextNodeCommand(PassRefPtr<Text> node, unsigned offset, const String& text)
: SimpleEditCommand(node->document())
, m_node(node)
, m_offset(offset)
, m_text(text)
{
ASSERT(m_node);
- ASSERT(offset >= 0);
- ASSERT(!text.isEmpty());
+ ASSERT(m_offset <= m_node->length());
+ ASSERT(!m_text.isEmpty());
}
void InsertIntoTextNodeCommand::doApply()
{
- ExceptionCode ec = 0;
+ ExceptionCode ec;
m_node->insertData(m_offset, m_text, ec);
- ASSERT(ec == 0);
}
void InsertIntoTextNodeCommand::doUnapply()
{
- ExceptionCode ec = 0;
+ ExceptionCode ec;
m_node->deleteData(m_offset, m_text.length(), ec);
- ASSERT(ec == 0);
}
} // namespace WebCore
