Commit 6442e7dc authored by yosin@chromium.org's avatar yosin@chromium.org

Ignore visibility when checking whether Position is editable or not at DeletetSelectionCommand

This patch introduces |lastEditablePositionBeforePositionInRoot()| to get
to editable |Position| instead of |VisiblePosition| for making
DeletetSelectionCommand to work editable position regardless its visibility.

The root cause of issue 339187 is |DeleteSelectionCommand| shrinks end point of 
selection range, start position is start of "bar", which is hidden editable, 
and end position is "foo", which isn't editable, to editable position but 
|DeleteSelectionCommand| checks both editability and visibility, then it gets 
null position since end point is hidden. 


BUG=339187
TEST=LayoutTests/editing/execCommand/delete-hidden-crash.html

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

git-svn-id: svn://svn.chromium.org/blink/trunk@176497 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 6690170a
<!DOCTYPE html>
<html>
<head>
<script>
if (window.testRunner)
testRunner.dumpAsText();
onload = function() {
document.execCommand('SelectAll')
span = document.getElementsByTagName('span')[0];
span.contentEditable = 'true';
span.textContent = 'bar';
document.execCommand('InsertText', false, '1');
document.body.textContent = 'PASS if Blink doesn\'t crash.';
};
</script>
<style>
* { visibility:visible; }
.inline { display:inline; }
*:only-child { visibility:hidden; }
</style>
</head>
<body>
<table>
<tr><td><span><table></table><span></span></span></td></tr>
<tr><td><td class="inline">foo</td></tr>
</table>
</body>
</html>
...@@ -166,7 +166,7 @@ void DeleteSelectionCommand::initializePositionData() ...@@ -166,7 +166,7 @@ void DeleteSelectionCommand::initializePositionData()
ASSERT(isEditablePosition(start, ContentIsEditable, DoNotUpdateStyle)); ASSERT(isEditablePosition(start, ContentIsEditable, DoNotUpdateStyle));
if (!isEditablePosition(end, ContentIsEditable, DoNotUpdateStyle)) if (!isEditablePosition(end, ContentIsEditable, DoNotUpdateStyle))
end = lastEditableVisiblePositionBeforePositionInRoot(end, highestEditableRoot(start)).deepEquivalent(); end = lastEditablePositionBeforePositionInRoot(end, highestEditableRoot(start));
m_upstreamStart = start.upstream(); m_upstreamStart = start.upstream();
m_downstreamStart = start.downstream(); m_downstreamStart = start.downstream();
......
...@@ -284,17 +284,22 @@ VisiblePosition firstEditableVisiblePositionAfterPositionInRoot(const Position& ...@@ -284,17 +284,22 @@ VisiblePosition firstEditableVisiblePositionAfterPositionInRoot(const Position&
} }
VisiblePosition lastEditableVisiblePositionBeforePositionInRoot(const Position& position, Node* highestRoot) VisiblePosition lastEditableVisiblePositionBeforePositionInRoot(const Position& position, Node* highestRoot)
{
return VisiblePosition(lastEditablePositionBeforePositionInRoot(position, highestRoot));
}
Position lastEditablePositionBeforePositionInRoot(const Position& position, Node* highestRoot)
{ {
// When position falls after highestRoot, the result is easy to compute. // When position falls after highestRoot, the result is easy to compute.
if (comparePositions(position, lastPositionInNode(highestRoot)) == 1) if (comparePositions(position, lastPositionInNode(highestRoot)) == 1)
return VisiblePosition(lastPositionInNode(highestRoot)); return lastPositionInNode(highestRoot);
Position editablePosition = position; Position editablePosition = position;
if (position.deprecatedNode()->treeScope() != highestRoot->treeScope()) { if (position.deprecatedNode()->treeScope() != highestRoot->treeScope()) {
Node* shadowAncestor = highestRoot->treeScope().ancestorInThisScope(editablePosition.deprecatedNode()); Node* shadowAncestor = highestRoot->treeScope().ancestorInThisScope(editablePosition.deprecatedNode());
if (!shadowAncestor) if (!shadowAncestor)
return VisiblePosition(); return Position();
editablePosition = firstPositionInOrBeforeNode(shadowAncestor); editablePosition = firstPositionInOrBeforeNode(shadowAncestor);
} }
...@@ -303,9 +308,8 @@ VisiblePosition lastEditableVisiblePositionBeforePositionInRoot(const Position& ...@@ -303,9 +308,8 @@ VisiblePosition lastEditableVisiblePositionBeforePositionInRoot(const Position&
editablePosition = isAtomicNode(editablePosition.deprecatedNode()) ? positionInParentBeforeNode(*editablePosition.deprecatedNode()) : previousVisuallyDistinctCandidate(editablePosition); editablePosition = isAtomicNode(editablePosition.deprecatedNode()) ? positionInParentBeforeNode(*editablePosition.deprecatedNode()) : previousVisuallyDistinctCandidate(editablePosition);
if (editablePosition.deprecatedNode() && editablePosition.deprecatedNode() != highestRoot && !editablePosition.deprecatedNode()->isDescendantOf(highestRoot)) if (editablePosition.deprecatedNode() && editablePosition.deprecatedNode() != highestRoot && !editablePosition.deprecatedNode()->isDescendantOf(highestRoot))
return VisiblePosition(); return Position();
return editablePosition;
return VisiblePosition(editablePosition);
} }
// FIXME: The method name, comment, and code say three different things here! // FIXME: The method name, comment, and code say three different things here!
......
...@@ -144,6 +144,8 @@ inline Position lastPositionInOrAfterNode(Node* node) ...@@ -144,6 +144,8 @@ inline Position lastPositionInOrAfterNode(Node* node)
return editingIgnoresContent(node) ? positionAfterNode(node) : lastPositionInNode(node); return editingIgnoresContent(node) ? positionAfterNode(node) : lastPositionInNode(node);
} }
Position lastEditablePositionBeforePositionInRoot(const Position&, Node*);
// comparision functions on Position // comparision functions on Position
int comparePositions(const Position&, const Position&); int comparePositions(const Position&, const Position&);
......
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