Skip to content

GitLab

T
tangled
Commit 46779daf authored by yutak@chromium.org's avatar yutak@chromium.org
Browse files
Options

Implement Range-based selection expansion. 

This patch simplifies Selection.addRange()'s implementation by expanding
the selection based on DOM Ranges instead of VisiblePositions.

There is a slight behavior change caused by this shift of data structures
we use: ranges which are discrete in the DOM tree but visually adjacent will
be rejected after this patch is landed. However, I don't think this will be
a huge problem, since (1) API behavior relying on visual positions can be
tricky to users because they are Blink-internal only information; and
(2) users wishing to connect the regions can (and probably should) specify
a range which overlaps with the current selection.

BUG=346613

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

git-svn-id: svn://svn.chromium.org/blink/trunk@170640 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 4db7bf3a
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 514 additions and 24 deletions
third_party/WebKit/LayoutTests/editing/selection/addRange-expected.txt
View file @ 46779daf
CONSOLE ERROR: Discontiguous selection is not supported.
CONSOLE ERROR: Discontiguous selection is not supported.
layer at (0,0) size 800x600 layer at (0,0) size 800x600
RenderView at (0,0) size 800x600 RenderView at (0,0) size 800x600
layer at (0,0) size 800x600 layer at (0,0) size 800x600
......
third_party/WebKit/LayoutTests/editing/selection/addRange-merging.html 0 → 100644
View file @ 46779daf
<!DOCTYPE html>
<html>
<head>
<title>Range merging with Selection.addRange()</title>
<script src="../../resources/js-test.js"></script>
</head>
<body>
<script>
description('Selection.addRange() should properly merge intersecting (and don\'t merge discrete) ranges.');
var selection = window.getSelection();
// Utility functions:
function createPosition(container, offset)
{
return {'container': container, 'offset': offset};
}
function createRange(startPosition, endPosition)
{
var range = new Range();
range.setStart(startPosition.container, startPosition.offset);
range.setEnd(endPosition.container, endPosition.offset);
return range;
}
function nodeToString(node)
{
switch (node.nodeType) {
case Node.ELEMENT_NODE:
return '[<' + node.tagName + '>: #' + node.id + ']';
case Node.TEXT_NODE:
return '[Text: ' + node.data + ']';
default:
return node.toString();
}
}
function positionToString(position)
{
return '(' + nodeToString(position.container) + ', ' + position.offset + ')';
}
function selectionShouldBe(expectedStartPosition, expectedEndPosition)
{
var range = selection.getRangeAt(0);
var actualStartPosition = createPosition(range.startContainer, range.startOffset);
var actualEndPosition = createPosition(range.endContainer, range.endOffset);
if (actualStartPosition.container === expectedStartPosition.container
&& actualStartPosition.offset === expectedStartPosition.offset
&& actualEndPosition.container === expectedEndPosition.container
&& actualEndPosition.offset === expectedEndPosition.offset) {
testPassed('Selection was: start = ' + positionToString(expectedStartPosition) + ', end = ' + positionToString(expectedEndPosition));
} else {
testFailed('Selection should be: start = ' + positionToString(expectedStartPosition) + ', end = ' + positionToString(expectedEndPosition) + '\nbut was: start = ' + positionToString(actualStartPosition) + ', end = ' + positionToString(actualEndPosition));
}
}
function runSingleTest(testFunction, initializePositionsFunction, containerIsEditable)
{
selection.removeAllRanges();
var container = document.createElement('div');
container.id = 'container';
if (containerIsEditable)
container.contentEditable = true;
document.body.appendChild(container);
var positions = initializePositionsFunction(container);
debug('Running: ' + testFunction.name + ' (initializePositionsFunction = ' + initializePositionsFunction.name + ', containerIsEditable = ' + containerIsEditable + ')');
testFunction(positions);
document.body.removeChild(container);
}
// Actual tests:
// To have better coverage over the possible code paths, each test is parametarized over four document positions;
// these positions are guaranteed to be ordered in the document order, but each position may vary in each test run.
//
// You can assume the selection is cleared before each test run.
function testExpandLeftToRight(positions)
{
selection.addRange(createRange(positions[0], positions[2]));
selection.addRange(createRange(positions[1], positions[3]));
selectionShouldBe(positions[0], positions[3]);
}
function testExpandRightToLeft(positions)
{
selection.addRange(createRange(positions[1], positions[3]));
selection.addRange(createRange(positions[0], positions[2]));
selectionShouldBe(positions[0], positions[3]);
}
function testExpandLeftToRightAdjacent(positions)
{
selection.addRange(createRange(positions[1], positions[2]));
selection.addRange(createRange(positions[2], positions[3]));
selectionShouldBe(positions[1], positions[3]);
}
function testExpandRightToLeftAdjacent(positions)
{
selection.addRange(createRange(positions[1], positions[2]));
selection.addRange(createRange(positions[0], positions[1]));
selectionShouldBe(positions[0], positions[2]);
}
function testExpandBothEnds(positions)
{
selection.addRange(createRange(positions[1], positions[2]));
selection.addRange(createRange(positions[0], positions[3]));
selectionShouldBe(positions[0], positions[3]);
}
function testDontExpand(positions)
{
selection.addRange(createRange(positions[0], positions[3]));
selection.addRange(createRange(positions[1], positions[2]));
selectionShouldBe(positions[0], positions[3]);
}
function testAddSameRange(positions)
{
selection.addRange(createRange(positions[1], positions[2]));
selection.addRange(createRange(positions[1], positions[2]));
selectionShouldBe(positions[1], positions[2]);
}
function testRejectDistantRangeAtRight(positions)
{
selection.addRange(createRange(positions[0], positions[1]));
selection.addRange(createRange(positions[2], positions[3]));
selectionShouldBe(positions[0], positions[1]);
}
function testRejectDistantRangeAtLeft(positions)
{
selection.addRange(createRange(positions[2], positions[3]));
selection.addRange(createRange(positions[0], positions[1]));
selectionShouldBe(positions[2], positions[3]);
}
function testRejectDistantCollapsedRangeAtRight(positions)
{
selection.addRange(createRange(positions[0], positions[1]));
selection.addRange(createRange(positions[2], positions[2]));
selectionShouldBe(positions[0], positions[1]);
}
function testRejectDistantCollapsedRangeAtLeft(positions)
{
selection.addRange(createRange(positions[2], positions[3]));
selection.addRange(createRange(positions[1], positions[1]));
selectionShouldBe(positions[2], positions[3]);
}
// Position initializers:
// Each initializer function takes an argument |container| which denotes the root element which can be filled with
// arbitrary contents. This element is created and added to the document before each test run, and removed from
// the document after each test run.
function initializeTextPositions(container)
{
container.innerHTML = '12345';
var text = container.firstChild;
return [createPosition(text, 1), createPosition(text, 2), createPosition(text, 3), createPosition(text, 4)];
}
function initializeOuterElementPositions(container)
{
container.innerHTML = '<span id="a">1</span><span id="b">2</span><span id="c">3</span><span id="d">4</span><span id="e">5</span>';
return [createPosition(container, 1), createPosition(container, 2), createPosition(container, 3), createPosition(container, 4)];
}
function initializeInnerElementPositions(container)
{
container.innerHTML = '<span id="a">1</span><span id="b">2</span><span id="c">3</span><span id="d">4</span><span id="e">5</span>';
return [createPosition(container.childNodes[1], 0), createPosition(container.childNodes[2], 0), createPosition(container.childNodes[3], 0), createPosition(container.childNodes[4], 0)];
}
function initializeVisiblyEquivalentPositionsBeforeNodes(container)
{
container.innerHTML = '<span id="a"><span id="b"><span id="c"></span></span></span>';
return [createPosition(container, 0), createPosition(container.firstChild, 0), createPosition(container.firstChild.firstChild, 0), createPosition(container.firstChild.firstChild.firstChild, 0)];
}
function initializeVisiblyEquivalentPositionsAfterNodes(container)
{
container.innerHTML = '<span id="a"><span id="b"><span id="c"></span></span></span>';
return [createPosition(container.firstChild.firstChild.firstChild, 0), createPosition(container.firstChild.firstChild, 1), createPosition(container.firstChild, 1), createPosition(container, 1)];
}
var tests = [
testExpandLeftToRight,
testExpandRightToLeft,
testExpandLeftToRightAdjacent,
testExpandRightToLeftAdjacent,
testExpandBothEnds,
testDontExpand,
testAddSameRange,
testRejectDistantRangeAtRight,
testRejectDistantRangeAtLeft,
testRejectDistantCollapsedRangeAtRight,
testRejectDistantCollapsedRangeAtLeft
];
var positionInitializers = [
initializeTextPositions,
initializeOuterElementPositions,
initializeInnerElementPositions,
initializeVisiblyEquivalentPositionsBeforeNodes,
initializeVisiblyEquivalentPositionsAfterNodes
];
tests.forEach(function (testFunction) {
positionInitializers.forEach(function (initializePositionsFunction) {
[false, true].forEach(function (containerIsEditable) {
runSingleTest(testFunction, initializePositionsFunction, containerIsEditable);
});
});
});
</script>
</body>
</html>
third_party/WebKit/LayoutTests/editing/text-iterator/findString-start-search-after-selection-expected.txt
View file @ 46779daf
CONSOLE ERROR: Discontiguous selection is not supported.
The _before_selection_ word is before the selection, so we shouldn't be able to find it if span_to_select is selected. The _before_selection_ word is before the selection, so we shouldn't be able to find it if span_to_select is selected.
The _in_selection_ word is in the selection and we should always be able to find it. The _in_selection_ word is in the selection and we should always be able to find it.
The _after_selection_ word is after the selection and we should always be able to find that too. The _after_selection_ word is after the selection and we should always be able to find that too.
......
third_party/WebKit/LayoutTests/platform/win/editing/selection/addRange-expected.txt
View file @ 46779daf
CONSOLE ERROR: Discontiguous selection is not supported.
CONSOLE ERROR: Discontiguous selection is not supported.
layer at (0,0) size 800x600 layer at (0,0) size 800x600
RenderView at (0,0) size 800x600 RenderView at (0,0) size 800x600
layer at (0,0) size 800x600 layer at (0,0) size 800x600
......
third_party/WebKit/Source/core/page/DOMSelection.cpp
View file @ 46779daf
...@@ -402,7 +402,7 @@ void DOMSelection::addRange(Range* newRange) ...@@ -402,7 +402,7 @@ void DOMSelection::addRange(Range* newRange)
return; return;
} }
RefPtr<Range> originalRange = selection.selection().toNormalizedRange(); RefPtr<Range> originalRange = selection.firstRange();
if (originalRange->startContainer()->document() != newRange->startContainer()->document()) { if (originalRange->startContainer()->document() != newRange->startContainer()->document()) {
addConsoleError("The given range does not belong to the current selection's document."); addConsoleError("The given range does not belong to the current selection's document.");
...@@ -413,30 +413,22 @@ void DOMSelection::addRange(Range* newRange) ...@@ -413,30 +413,22 @@ void DOMSelection::addRange(Range* newRange)
return; return;
} }
// FIXME: Emit a console error if the combined ranges would form a discontiguous selection. if (originalRange->compareBoundaryPoints(Range::START_TO_END, newRange, ASSERT_NO_EXCEPTION) < 0
if (newRange->compareBoundaryPoints(Range::START_TO_START, originalRange.get(), ASSERT_NO_EXCEPTION) == -1) { || newRange->compareBoundaryPoints(Range::START_TO_END, originalRange.get(), ASSERT_NO_EXCEPTION) < 0) {
// We don't support discontiguous selection. We don't do anything if newRange and originalRange don't intersect. addConsoleError("Discontiguous selection is not supported.");
if (newRange->compareBoundaryPoints(Range::START_TO_END, originalRange.get(), ASSERT_NO_EXCEPTION) > -1) { return;
if (newRange->compareBoundaryPoints(Range::END_TO_END, originalRange.get(), ASSERT_NO_EXCEPTION) == -1) {
// The original originalRange and newRange intersect.
selection.setSelection(VisibleSelection(newRange->startPosition(), originalRange->endPosition(), DOWNSTREAM));
} else {
// newRange contains the original originalRange.
selection.setSelection(VisibleSelection(newRange));
}
}
} else {
// We don't support discontiguous selection. We don't do anything if newRange and originalRange don't intersect.
if (newRange->compareBoundaryPoints(Range::END_TO_START, originalRange.get(), ASSERT_NO_EXCEPTION) < 1) {
if (newRange->compareBoundaryPoints(Range::END_TO_END, originalRange.get(), ASSERT_NO_EXCEPTION) == -1) {
// The original range contains newRange.
selection.setSelection(VisibleSelection(originalRange.get()));
} else {
// The original range and r intersect.
selection.setSelection(VisibleSelection(originalRange->startPosition(), newRange->endPosition(), DOWNSTREAM));
}
}
} }
// FIXME: "Merge the ranges if they intersect" is Blink-specific behavior; other browsers supporting discontiguous
// selection (obviously) keep each Range added and return it in getRangeAt(). But it's unclear if we can really
// do the same, since we don't support discontiguous selection. Further discussions at
// <https://code.google.com/p/chromium/issues/detail?id=353069>.
Range* start = originalRange->compareBoundaryPoints(Range::START_TO_START, newRange, ASSERT_NO_EXCEPTION) < 0 ? originalRange.get() : newRange;
Range* end = originalRange->compareBoundaryPoints(Range::END_TO_END, newRange, ASSERT_NO_EXCEPTION) < 0 ? newRange : originalRange.get();
RefPtr<Range> merged = Range::create(originalRange->startContainer()->document(), start->startContainer(), start->startOffset(), end->endContainer(), end->endOffset());
EAffinity affinity = selection.selection().affinity();
selection.setSelectedRange(merged.get(), affinity);
} }
void DOMSelection::deleteFromDocument() void DOMSelection::deleteFromDocument()
......
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