Skip to content

GitLab

T
tangled
Commit 9d10de02 authored by tanvir.rizvi's avatar tanvir.rizvi Committed by Commit Bot
Browse files
Options

InsertOrderedList creates incorrect selection 

InsertOrderList command creates a numbered
ordered list for the selection.
We try to restore the original selection
by storing the index of the original start
and end positions.
There are cases where after list command
is applied the index changes, causing wrong
selection shown.
This CL tries to fix those cases by
introducing a TextIteratorBehavior,
which suppresses the extra new line
added for a <p> tag.


Bug: 345935
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I8019f6176f859f4d2c19030e08e1836b1f05a27d
Reviewed-on: https://chromium-review.googlesource.com/850252
Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532341}
parent 6da6970d
Hide whitespace changes
Inline Side-by-side
Showing with 80 additions and 20 deletions
chrome/browser/resources/chromeos/select_to_speak/select_to_speak_keystroke_selection_test.extjs
View file @ 9d10de02
...@@ -142,7 +142,7 @@ TEST_F('SelectToSpeakKeystrokeSelectionTest', 'SpeaksAcrossNodesAtKeystroke', ...@@ -142,7 +142,7 @@ TEST_F('SelectToSpeakKeystrokeSelectionTest', 'SpeaksAcrossNodesAtKeystroke',
let lastNode = this.findTextNode(desktop, ' text'); let lastNode = this.findTextNode(desktop, ' text');
chrome.automation.setDocumentSelection({ chrome.automation.setDocumentSelection({
anchorObject: firstNode, anchorOffset: 0, anchorObject: firstNode, anchorOffset: 0,
focusObject: lastNode, focusOffset: 6 focusObject: lastNode, focusOffset: 5
}); });
}, 'This is some bold text'); }, 'This is some bold text');
}); });
...@@ -156,7 +156,7 @@ TEST_F('SelectToSpeakKeystrokeSelectionTest', ...@@ -156,7 +156,7 @@ TEST_F('SelectToSpeakKeystrokeSelectionTest',
let lastNode = this.findTextNode(desktop, 'This is some '); let lastNode = this.findTextNode(desktop, 'This is some ');
let firstNode = this.findTextNode(desktop, ' text'); let firstNode = this.findTextNode(desktop, ' text');
chrome.automation.setDocumentSelection({ chrome.automation.setDocumentSelection({
anchorObject: firstNode, anchorOffset: 6, anchorObject: firstNode, anchorOffset: 5,
focusObject: lastNode, focusOffset: 0 focusObject: lastNode, focusOffset: 0
}); });
}, 'This is some bold text'); }, 'This is some bold text');
......
third_party/WebKit/LayoutTests/editing/execCommand/insert-ordered-list-create-correct-selection.html 0 → 100644
View file @ 9d10de02
<!doctype html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../assert_selection.js"></script>
<script>
// Selection should be restored correctly based
// on previous selection after list is applied.
selection_test(
[
'<div contenteditable>',
'^<p>A</p>',
'<p>B</p>',
'<p>C</p>',
'<p>D</p>',
'<p>E</p>',
'<p>F|</p>',
'<p>This should not be highlighted</p>',
'</div>'
],
'insertOrderedList',
[
'<div contenteditable>',
'<p>',
'<ol>',
'<li>^A</li>',
'<li>B</li>',
'<li>C</li>',
'<li>D</li>',
'<li>E</li>',
'<li>F|</li>',
'</ol>',
'</p>',
'<p>This should not be highlighted</p>',
'</div>'
],
'This is test for correct selection');
</script>
third_party/WebKit/LayoutTests/external/wpt/editing/run/formatblock-expected.txt
View file @ 9d10de02
...@@ -2229,7 +2229,7 @@ PASS [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandState(" ...@@ -2229,7 +2229,7 @@ PASS [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandState("
FAIL [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandValue("formatblock") before assert_equals: Wrong result returned expected "p" but got "" FAIL [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandValue("formatblock") before assert_equals: Wrong result returned expected "p" but got ""
PASS [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandIndeterm("formatblock") after PASS [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandIndeterm("formatblock") after
PASS [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandState("formatblock") after PASS [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandState("formatblock") after
FAIL [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandValue("formatblock") after assert_equals: Wrong result returned expected "p" but got "" FAIL [["formatblock","<blockquote>"]] "<p>[foo<p>bar]<p>baz" queryCommandValue("formatblock") after assert_equals: Wrong result returned expected "p" but got "blockquote"
FAIL [["formatblock","<blockquote>"]] "<section>[foo]</section>": execCommand("formatblock", false, "<blockquote>") return value assert_equals: expected false but got true FAIL [["formatblock","<blockquote>"]] "<section>[foo]</section>": execCommand("formatblock", false, "<blockquote>") return value assert_equals: expected false but got true
PASS [["formatblock","<blockquote>"]] "<section>[foo]</section>" checks for modifications to non-editable content PASS [["formatblock","<blockquote>"]] "<section>[foo]</section>" checks for modifications to non-editable content
FAIL [["formatblock","<blockquote>"]] "<section>[foo]</section>" compare innerHTML assert_equals: Unexpected innerHTML (after normalizing inline style) expected "<section>foo</section>" but got "<blockquote>foo</blockquote>" FAIL [["formatblock","<blockquote>"]] "<section>[foo]</section>" compare innerHTML assert_equals: Unexpected innerHTML (after normalizing inline style) expected "<section>foo</section>" but got "<blockquote>foo</blockquote>"
......
third_party/WebKit/Source/core/editing/EditingUtilities.cpp
View file @ 9d10de02
...@@ -1479,9 +1479,13 @@ int IndexForVisiblePosition(const VisiblePosition& visible_position, ...@@ -1479,9 +1479,13 @@ int IndexForVisiblePosition(const VisiblePosition& visible_position,
EphemeralRange range(Position::FirstPositionInNode(*scope), EphemeralRange range(Position::FirstPositionInNode(*scope),
p.ParentAnchoredEquivalent()); p.ParentAnchoredEquivalent());
return TextIterator::RangeLength( const TextIteratorBehavior& behavior =
range.StartPosition(), range.EndPosition(), TextIteratorBehavior::Builder(
TextIteratorBehavior::AllVisiblePositionsRangeLengthBehavior()); TextIteratorBehavior::AllVisiblePositionsRangeLengthBehavior())
.SetSuppressesExtraNewlineEmission(true)
.Build();
return TextIterator::RangeLength(range.StartPosition(), range.EndPosition(),
behavior);
} }
EphemeralRange MakeRange(const VisiblePosition& start, EphemeralRange MakeRange(const VisiblePosition& start,
...@@ -1534,7 +1538,8 @@ VisiblePosition VisiblePositionForIndex(int index, ContainerNode* scope) { ...@@ -1534,7 +1538,8 @@ VisiblePosition VisiblePositionForIndex(int index, ContainerNode* scope) {
DocumentLifecycle::DisallowTransitionScope disallow_transition( DocumentLifecycle::DisallowTransitionScope disallow_transition(
scope->GetDocument().Lifecycle()); scope->GetDocument().Lifecycle());
EphemeralRange range = PlainTextRange(index).CreateRangeForSelection(*scope); EphemeralRange range =
PlainTextRange(index).CreateRangeForSelectionIndexing(*scope);
// Check for an invalid index. Certain editing operations invalidate indices // Check for an invalid index. Certain editing operations invalidate indices
// because of problems with // because of problems with
// TextIteratorEmitsCharactersBetweenAllVisiblePositions. // TextIteratorEmitsCharactersBetweenAllVisiblePositions.
......
third_party/WebKit/Source/core/editing/PlainTextRange.cpp
View file @ 9d10de02
...@@ -53,16 +53,37 @@ PlainTextRange::PlainTextRange(int start, int end) : start_(start), end_(end) { ...@@ -53,16 +53,37 @@ PlainTextRange::PlainTextRange(int start, int end) : start_(start), end_(end) {
} }
EphemeralRange PlainTextRange::CreateRange(const ContainerNode& scope) const { EphemeralRange PlainTextRange::CreateRange(const ContainerNode& scope) const {
return CreateRangeFor(scope, kForGeneric); const TextIteratorBehavior& behavior =
TextIteratorBehavior::Builder()
.SetEmitsObjectReplacementCharacter(true)
.Build();
return CreateRangeFor(scope, behavior);
} }
EphemeralRange PlainTextRange::CreateRangeForSelection( EphemeralRange PlainTextRange::CreateRangeForSelection(
const ContainerNode& scope) const { const ContainerNode& scope) const {
return CreateRangeFor(scope, kForSelection); const TextIteratorBehavior& behavior =
TextIteratorBehavior::Builder()
.SetEmitsObjectReplacementCharacter(true)
.SetEmitsCharactersBetweenAllVisiblePositions(true)
.Build();
return CreateRangeFor(scope, behavior);
}
EphemeralRange PlainTextRange::CreateRangeForSelectionIndexing(
const ContainerNode& scope) const {
const TextIteratorBehavior& behavior =
TextIteratorBehavior::Builder()
.SetEmitsObjectReplacementCharacter(true)
.SetEmitsCharactersBetweenAllVisiblePositions(true)
.SetSuppressesExtraNewlineEmission(true)
.Build();
return CreateRangeFor(scope, behavior);
} }
EphemeralRange PlainTextRange::CreateRangeFor(const ContainerNode& scope, EphemeralRange PlainTextRange::CreateRangeFor(
GetRangeFor get_range_for) const { const ContainerNode& scope,
const TextIteratorBehavior& behavior) const {
DCHECK(IsNotNull()); DCHECK(IsNotNull());
size_t doc_text_position = 0; size_t doc_text_position = 0;
...@@ -71,13 +92,6 @@ EphemeralRange PlainTextRange::CreateRangeFor(const ContainerNode& scope, ...@@ -71,13 +92,6 @@ EphemeralRange PlainTextRange::CreateRangeFor(const ContainerNode& scope,
Position text_run_start_position; Position text_run_start_position;
Position text_run_end_position; Position text_run_end_position;
const TextIteratorBehavior& behavior =
TextIteratorBehavior::Builder()
.SetEmitsObjectReplacementCharacter(true)
.SetEmitsCharactersBetweenAllVisiblePositions(get_range_for ==
kForSelection)
.Build();
auto range = EphemeralRange::RangeOfContents(scope); auto range = EphemeralRange::RangeOfContents(scope);
TextIterator it(range.StartPosition(), range.EndPosition(), behavior); TextIterator it(range.StartPosition(), range.EndPosition(), behavior);
......
third_party/WebKit/Source/core/editing/PlainTextRange.h
View file @ 9d10de02
...@@ -36,6 +36,7 @@ namespace blink { ...@@ -36,6 +36,7 @@ namespace blink {
class ContainerNode; class ContainerNode;
class Range; class Range;
class TextIteratorBehavior;
class CORE_EXPORT PlainTextRange { class CORE_EXPORT PlainTextRange {
STACK_ALLOCATED(); STACK_ALLOCATED();
...@@ -63,6 +64,8 @@ class CORE_EXPORT PlainTextRange { ...@@ -63,6 +64,8 @@ class CORE_EXPORT PlainTextRange {
EphemeralRange CreateRange(const ContainerNode& scope) const; EphemeralRange CreateRange(const ContainerNode& scope) const;
EphemeralRange CreateRangeForSelection(const ContainerNode& scope) const; EphemeralRange CreateRangeForSelection(const ContainerNode& scope) const;
EphemeralRange CreateRangeForSelectionIndexing(
const ContainerNode& scope) const;
static PlainTextRange Create(const ContainerNode& scope, static PlainTextRange Create(const ContainerNode& scope,
const EphemeralRange&); const EphemeralRange&);
...@@ -71,8 +74,8 @@ class CORE_EXPORT PlainTextRange { ...@@ -71,8 +74,8 @@ class CORE_EXPORT PlainTextRange {
private: private:
PlainTextRange& operator=(const PlainTextRange&) = delete; PlainTextRange& operator=(const PlainTextRange&) = delete;
enum GetRangeFor { kForGeneric, kForSelection }; EphemeralRange CreateRangeFor(const ContainerNode& scope,
EphemeralRange CreateRangeFor(const ContainerNode& scope, GetRangeFor) const; const TextIteratorBehavior&) const;
const size_t start_; const size_t start_;
const size_t end_; const size_t end_;
......
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