Skip to content

GitLab

T
tangled
Commit 83a3a9bf authored by yoichio@chromium.org's avatar yoichio@chromium.org
Browse files
Options

reset VisibleSelection at the Undo command. 

Sice UndoManager doesn't manage DOM mutations outside of execCommand, the Undo command sometimes set broken VisibleSelection which is valid when it was recorded to the Undo steps but selection.deleteFromDocument broke that in this test case.

This CL validates VisibleSelection when it is used to restore Selection at Undo.
The main target VisibleSelection to be validate is that has any VisiblePosition which is not in the document already or which offset violates its nodes length because of "Unmagaed" DOM mutations.

BUG=369759

Review URL: https://codereview.chromium.org/315493004

git-svn-id: svn://svn.chromium.org/blink/trunk@175647 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 7cbc9bd3
Hide whitespace changes
Inline Side-by-side
Showing with 76 additions and 24 deletions
third_party/WebKit/LayoutTests/editing/undo/crash-delete-from-document.html 0 → 100644
View file @ 83a3a9bf
<!DOCTYPE html>
<head>
<script>
if (window.testRunner)
testRunner.dumpAsText();
window.onload = function () {
document.designMode = 'on';
document.body.textContent = 'aaaa';
document.execCommand('SelectAll');
document.execCommand('Italic');
window.getSelection().deleteFromDocument();
document.execCommand('Undo');
window.getSelection().deleteFromDocument();
document.body.textContent = 'PASS if Blink doesn\'t crash.';
};
</script>
</head>
<body>
</body>
</html>
\ No newline at end of file
third_party/WebKit/Source/core/dom/Node.cpp
View file @ 83a3a9bf
......@@ -2558,6 +2558,28 @@ void Node::trace(Visitor* visitor)
EventTarget::trace(visitor);
}
unsigned Node::lengthOfContents() const
{
// This switch statement must be consistent with that of Range::processContentsBetweenOffsets.
switch (nodeType()) {
case Node::TEXT_NODE:
case Node::CDATA_SECTION_NODE:
case Node::COMMENT_NODE:
return toCharacterData(this)->length();
case Node::PROCESSING_INSTRUCTION_NODE:
return toProcessingInstruction(this)->data().length();
case Node::ELEMENT_NODE:
case Node::ATTRIBUTE_NODE:
case Node::DOCUMENT_NODE:
case Node::DOCUMENT_FRAGMENT_NODE:
return toContainerNode(this)->countChildren();
case Node::DOCUMENT_TYPE_NODE:
return 0;
}
ASSERT_NOT_REACHED();
return 0;
}
} // namespace WebCore
#ifndef NDEBUG
......
third_party/WebKit/Source/core/dom/Node.h
View file @ 83a3a9bf
......@@ -674,6 +674,8 @@ public:
virtual void trace(Visitor*) OVERRIDE;
unsigned lengthOfContents() const;
private:
enum NodeFlags {
HasRareDataFlag = 1,
......
third_party/WebKit/Source/core/dom/Range.cpp
View file @ 83a3a9bf
......@@ -548,28 +548,6 @@ static inline Node* childOfCommonRootBeforeOffset(Node* container, unsigned offs
return container;
}
static inline unsigned lengthOfContentsInNode(Node* node)
{
// This switch statement must be consistent with that of Range::processContentsBetweenOffsets.
switch (node->nodeType()) {
case Node::TEXT_NODE:
case Node::CDATA_SECTION_NODE:
case Node::COMMENT_NODE:
return toCharacterData(node)->length();
case Node::PROCESSING_INSTRUCTION_NODE:
return toProcessingInstruction(node)->data().length();
case Node::ELEMENT_NODE:
case Node::ATTRIBUTE_NODE:
case Node::DOCUMENT_NODE:
case Node::DOCUMENT_FRAGMENT_NODE:
return toContainerNode(node)->countChildren();
case Node::DOCUMENT_TYPE_NODE:
return 0;
}
ASSERT_NOT_REACHED();
return 0;
}
PassRefPtrWillBeRawPtr<DocumentFragment> Range::processContents(ActionType action, ExceptionState& exceptionState)
{
typedef WillBeHeapVector<RefPtrWillBeMember<Node> > NodeVector;
......@@ -619,7 +597,7 @@ PassRefPtrWillBeRawPtr<DocumentFragment> Range::processContents(ActionType actio
RefPtrWillBeRawPtr<Node> leftContents = nullptr;
if (originalStart.container() != commonRoot && commonRoot->contains(originalStart.container())) {
leftContents = processContentsBetweenOffsets(action, nullptr, originalStart.container(), originalStart.offset(), lengthOfContentsInNode(originalStart.container()), exceptionState);
leftContents = processContentsBetweenOffsets(action, nullptr, originalStart.container(), originalStart.offset(), originalStart.container()->lengthOfContents(), exceptionState);
leftContents = processAncestorsAndTheirSiblings(action, originalStart.container(), ProcessContentsForward, leftContents, commonRoot.get(), exceptionState);
}
......@@ -687,7 +665,7 @@ PassRefPtrWillBeRawPtr<Node> Range::processContentsBetweenOffsets(ActionType act
ASSERT(container);
ASSERT(startOffset <= endOffset);
// This switch statement must be consistent with that of lengthOfContentsInNode.
// This switch statement must be consistent with that of Node::lengthOfContents.
RefPtrWillBeRawPtr<Node> result = nullptr;
switch (container->nodeType()) {
case Node::TEXT_NODE:
......
third_party/WebKit/Source/core/editing/Editor.cpp
View file @ 83a3a9bf
......@@ -718,6 +718,7 @@ void Editor::unappliedEditing(PassRefPtrWillBeRawPtr<EditCommandComposition> cmd
dispatchEditableContentChangedEvents(cmd->startingRootEditableElement(), cmd->endingRootEditableElement());
VisibleSelection newSelection(cmd->startingSelection());
newSelection.validatePositionsIfNeeded();
changeSelectionAfterCommand(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);
m_lastEditCommand = nullptr;
......
third_party/WebKit/Source/core/editing/VisibleSelection.cpp
View file @ 83a3a9bf
......@@ -771,6 +771,28 @@ void VisibleSelection::trace(Visitor* visitor)
visitor->trace(m_changeObserver);
}
static bool isValidPosition(const Position& position)
{
if (!position.inDocument())
return false;
if (position.anchorType() != Position::PositionIsOffsetInAnchor)
return true;
if (position.offsetInContainerNode() < 0)
return false;
const unsigned offset = static_cast<unsigned>(position.offsetInContainerNode());
const unsigned nodeLength = position.anchorNode()->lengthOfContents();
return offset <= nodeLength;
}
void VisibleSelection::validatePositionsIfNeeded()
{
if (!isValidPosition(m_base) || !isValidPosition(m_extent) || !isValidPosition(m_start) || !isValidPosition(m_end))
validate();
}
#ifndef NDEBUG
void VisibleSelection::debugPosition() const
......
third_party/WebKit/Source/core/editing/VisibleSelection.h
View file @ 83a3a9bf
......@@ -132,6 +132,8 @@ public:
void trace(Visitor*);
void validatePositionsIfNeeded();
#ifndef NDEBUG
void debugPosition() const;
void formatForDebugger(char* buffer, unsigned length) 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