Skip to content

GitLab

T
tangled
Commit 8dfbb6f9 authored by Anupam Snigdha's avatar Anupam Snigdha Committed by Commit Bot
Browse files
Options

Fix selection movement when crossing editing boundaries. 

When caret is moved by line,|SelectionModifier::PreviousLinePosition|
and |SelectionModifier::NextLinePosition| both return the positions
that could be crossing editing boundaries as there is no editability
check in those APIs. In this patch we are matching other browsers in
caret movement when user presses up/down arrow keys to navigate
across editable boundaries to next/previous line and position the
caret inside an editable region so it is rendered properly.

Test: web_tests/editing/selection/select-line-up-down.html

Bug: 964504

Change-Id: I596e006f4dcc42db3b74d0f2ed577f8e0b27f96b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2556007
Commit-Queue: Anupam Snigdha <snianu@microsoft.com>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831110}
parent 001a4742
Hide whitespace changes
Inline Side-by-side
Showing with 125 additions and 2 deletions
third_party/blink/renderer/core/editing/selection_modifier_line.cc
View file @ 8dfbb6f9
...@@ -458,8 +458,18 @@ VisiblePosition SelectionModifier::PreviousLinePosition( ...@@ -458,8 +458,18 @@ VisiblePosition SelectionModifier::PreviousLinePosition(
line.AbsoluteLineDirectionPointToLocalPointInBlock( line.AbsoluteLineDirectionPointToLocalPointInBlock(
line_direction_point); line_direction_point);
if (auto position = if (auto position =
line.PositionForPoint(point_in_line, IsEditablePosition(p))) line.PositionForPoint(point_in_line, IsEditablePosition(p))) {
// If the current position is inside an editable position, then the next
// shouldn't end up inside non-editable as that would cross the editing
// boundaries which would be an invalid selection.
if (IsEditablePosition(p) &&
!IsEditablePosition(position.GetPosition())) {
return CreateVisiblePosition(
AdjustBackwardPositionToAvoidCrossingEditingBoundaries(
position, visible_position.DeepEquivalent()));
}
return CreateVisiblePosition(position); return CreateVisiblePosition(position);
}
} }
// Could not find a previous line. This means we must already be on the first // Could not find a previous line. This means we must already be on the first
...@@ -522,8 +532,18 @@ VisiblePosition SelectionModifier::NextLinePosition( ...@@ -522,8 +532,18 @@ VisiblePosition SelectionModifier::NextLinePosition(
line.AbsoluteLineDirectionPointToLocalPointInBlock( line.AbsoluteLineDirectionPointToLocalPointInBlock(
line_direction_point); line_direction_point);
if (auto position = if (auto position =
line.PositionForPoint(point_in_line, IsEditablePosition(p))) line.PositionForPoint(point_in_line, IsEditablePosition(p))) {
// If the current position is inside an editable position, then the next
// shouldn't end up inside non-editable as that would cross the editing
// boundaries which would be an invalid selection.
if (IsEditablePosition(p) &&
!IsEditablePosition(position.GetPosition())) {
return CreateVisiblePosition(
AdjustForwardPositionToAvoidCrossingEditingBoundaries(
position, visible_position.DeepEquivalent()));
}
return CreateVisiblePosition(position); return CreateVisiblePosition(position);
}
} }
// Could not find a next line. This means we must already be on the last line. // Could not find a next line. This means we must already be on the last line.
......
third_party/blink/web_tests/FlagExpectations/disable-layout-ng
View file @ 8dfbb6f9
...@@ -23,6 +23,9 @@ crbug.com/591099 editing/pasteboard/mathml-sanitizer-bypass.html [ Failure ] ...@@ -23,6 +23,9 @@ crbug.com/591099 editing/pasteboard/mathml-sanitizer-bypass.html [ Failure ]
### editing/selection/mouse/ ### editing/selection/mouse/
crbug.com/591099 editing/selection/mouse/drag_over_text_transform.html [ Crash ] crbug.com/591099 editing/selection/mouse/drag_over_text_transform.html [ Crash ]
### editing/selection/select-line-up-down.html
crbug.com/591099 editing/selection/select-line-up-down.html [ Failure ]
### external/wpt/css/CSS2/abspos/ ### external/wpt/css/CSS2/abspos/
crbug.com/591099 external/wpt/css/CSS2/abspos/hypothetical-box-dynamic.html [ Failure ] crbug.com/591099 external/wpt/css/CSS2/abspos/hypothetical-box-dynamic.html [ Failure ]
......
third_party/blink/web_tests/editing/selection/select-line-up-down.html 0 → 100644
View file @ 8dfbb6f9
<!DOCTYPE html>
<html>
<head>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../assert_selection.js"></script>
<script>
selection_test(
[
'<div contenteditable>',
'<div>line| 1</div>',
'<div><span contenteditable="false">line 2</span><br></div>',
'<div>line 3</div>',
'</div>',
],
selection => selection.modify('move', 'forward', 'line'),
[
'<div contenteditable>',
'<div>line 1</div>',
'<div><span contenteditable="false">line 2</span>|<br></div>',
'<div>line 3</div>',
'</div>',
],
'Down arrow selection should not cross editing boundaries.');
selection_test(
[
'<div contenteditable>',
'<div>line| 1</div>',
'<div>abc<span contenteditable="false">line 2</span><br></div>',
'<div>line 3</div>',
'</div>',
],
selection => selection.modify('move', 'forward', 'line'),
[
'<div contenteditable>',
'<div>line 1</div>',
'<div>abc|<span contenteditable="false">line 2</span><br></div>',
'<div>line 3</div>',
'</div>',
],
'Down arrow selection should not cross editing boundaries when content is present before non-editable.');
selection_test(
[
'<div contenteditable>',
'<div>line 1</div>',
'<div><span contenteditable="false">line 2</span><br></div>',
'<div>line| 3</div>',
'</div>',
],
selection => selection.modify('move', 'backward', 'line'),
[
'<div contenteditable>',
'<div>line 1</div>',
'<div><span contenteditable="false">line 2</span>|<br></div>',
'<div>line 3</div>',
'</div>',
],
'Up arrow selection should not cross editing boundaries.');
selection_test(
[
'<div contenteditable>',
'<div>line 1</div>',
'<div>abc<span contenteditable="false">line 2</span><br></div>',
'<div>line| 3</div>',
'</div>',
],
selection => selection.modify('move', 'backward', 'line'),
[
'<div contenteditable>',
'<div>line 1</div>',
'<div>abc|<span contenteditable="false">line 2</span><br></div>',
'<div>line 3</div>',
'</div>',
],
'Up arrow selection should not cross editing boundaries when content is present before non-editable.');
selection_test(
[
'<div contenteditable>',
'<div>line 1</div>',
'<div><span contenteditable="false">line 2</span>test<br></div>',
'<div>line| 3</div>',
'</div>',
],
selection => selection.modify('move', 'backward', 'line'),
[
'<div contenteditable>',
'<div>line 1</div>',
'<div><span contenteditable="false">line 2</span>|test<br></div>',
'<div>line 3</div>',
'</div>',
],
'Up arrow selection should not cross editing boundaries when editable content present around non-editable.');
</script>
</html>
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