Commit 350228ed authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

Make PreviousWordPosition() utilitize TextSegments class

This patch changes |PreviousWordPosition()| to utilize |TextSegments|
class to make |PreviousWordPosition()| work with LayoutNG.

This patch also reveals an existing bug (or feature?) in editing
boundary adjustments, as shown in the changed layout test.

Bug: 778507, 900060
Change-Id: Ic797c960715576dc85a946a7b34b0be568c19805
Reviewed-on: https://chromium-review.googlesource.com/c/1307056Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604152}
parent 6838dc55
......@@ -4651,6 +4651,9 @@ crbug.com/770232 [ Win10 ] fast/text/unicode-fallback-font.html [ Failure ]
crbug.com/764489 editing/execCommand/format-block-multiple-paragraphs.html [ Failure ]
crbug.com/764489 editing/execCommand/format-block-multiple-paragraphs-in-pre.html [ Failure ]
# Previous/NextWordPosition crossing editing boundaries.
crbug.com/900060 editing/selection/mixed-editability-8.html [ Failure ]
# Sheriff failures 2017-09-11
crbug.com/763975 [ Linux Mac Debug ] webaudio/internals/cycle-connection-gc.html [ Pass Failure ]
......
......@@ -60,5 +60,33 @@ selection_test(
'editable',
'</div>',
],
'Move backward backward');
'Move backward word');
selection_test(
[
'<div contenteditable>',
'editable',
'<table border="1" contenteditable="false"><tbody><tr><td>',
'<div style="display:inline-block;">',
'<span contenteditable="plaintext-only">e|ditable</span>',
'<span>static</span>',
'</div>',
'</td></tr></tbody></table>',
'editable',
'</div>',
],
selection => selection.modify('move', 'forward', 'word'),
[
'<div contenteditable>',
'editable',
'<table border="1" contenteditable="false"><tbody><tr><td>',
'<div style="display:inline-block;">',
'<span contenteditable="plaintext-only">editable|</span>',
'<span>static</span>',
'</div>',
'</td></tr></tbody></table>',
'editable',
'</div>',
],
'Move forward word');
</script>
......@@ -56,4 +56,28 @@ PositionInFlatTree TextSegments::FindBoundaryForward(
return last_position;
}
// static
PositionInFlatTree TextSegments::FindBoundaryBackward(
const PositionInFlatTree& position,
Finder* finder) {
DCHECK(position.IsNotNull());
PositionInFlatTree last_position = position;
for (const auto& inline_contents :
TextOffsetMapping::BackwardRangeOf(position)) {
const TextOffsetMapping mapping(inline_contents);
const String text = mapping.GetText();
const unsigned offset = last_position == position
? mapping.ComputeTextOffset(position)
: mapping.GetText().length();
const TextSegments::Finder::Position result = finder->Find(text, offset);
if (result.IsBefore())
return mapping.GetPositionBefore(result.Offset());
if (result.IsAfter())
return mapping.GetPositionAfter(result.Offset());
DCHECK(result.IsNone());
last_position = mapping.GetRange().StartPosition();
}
return last_position;
}
} // namespace blink
......@@ -51,13 +51,15 @@ class TextSegments final {
virtual Position Find(const String text, unsigned offset) = 0;
};
// TODO(editing-dev): We should have |FindBoundaryBackward()|.
// Returns a boundary position found by |finder| followed by |position|
// (inclusive). |finder| can be stateful or stateless.
static PositionInFlatTree FindBoundaryForward(
const PositionInFlatTree& position,
Finder* finder);
static PositionInFlatTree FindBoundaryBackward(
const PositionInFlatTree& position,
Finder* finder);
};
} // namespace blink
......
......@@ -110,19 +110,30 @@ PositionInFlatTree NextWordPositionInternal(
return TextSegments::FindBoundaryForward(position, &finder);
}
unsigned PreviousWordPositionBoundary(
const UChar* characters,
unsigned length,
unsigned offset,
BoundarySearchContextAvailability may_have_more_context,
bool& need_more_context) {
if (may_have_more_context &&
!StartOfLastWordBoundaryContext(characters, offset)) {
need_more_context = true;
return 0;
PositionInFlatTree PreviousWordPositionInternal(
const PositionInFlatTree& position) {
class Finder final : public TextSegments::Finder {
STACK_ALLOCATED();
private:
Position Find(const String text, unsigned offset) final {
DCHECK_LE(offset, text.length());
if (!offset || text.length() == 0)
return Position();
TextBreakIterator* it =
WordBreakIterator(text.Characters16(), text.length());
for (int runner = it->preceding(offset); runner != kTextBreakDone;
runner = it->preceding(runner)) {
// We stop searching when the character following the break is
// alphanumeric or underscore.
if (runner && (WTF::Unicode::IsAlphanumeric(text[runner]) ||
text[runner] == kLowLineCharacter))
return Position::Before(runner);
}
need_more_context = false;
return FindNextWordBackward(characters, length, offset);
return Position::Before(0);
}
} finder;
return TextSegments::FindBoundaryBackward(position, &finder);
}
unsigned StartWordBoundary(
......@@ -219,12 +230,24 @@ VisiblePosition NextWordPosition(const VisiblePosition& c) {
return CreateVisiblePosition(NextWordPosition(c.DeepEquivalent()));
}
PositionInFlatTreeWithAffinity PreviousWordPosition(
const PositionInFlatTree& start) {
const PositionInFlatTree prev = PreviousWordPositionInternal(start);
return AdjustBackwardPositionToAvoidCrossingEditingBoundaries(
PositionInFlatTreeWithAffinity(prev), start);
}
PositionWithAffinity PreviousWordPosition(const Position& start) {
const PositionInFlatTreeWithAffinity& prev =
PreviousWordPosition(ToPositionInFlatTree(start));
return ToPositionInDOMTreeWithAffinity(prev);
}
// TODO(xiaochengh): Remove this function. Change callers to use the
// Position version as it doesn't need canonical input position.
VisiblePosition PreviousWordPosition(const VisiblePosition& c) {
DCHECK(c.IsValid()) << c;
VisiblePosition prev =
CreateVisiblePosition(PreviousBoundary(c, PreviousWordPositionBoundary));
return AdjustBackwardPositionToAvoidCrossingEditingBoundaries(
prev, c.DeepEquivalent());
return CreateVisiblePosition(PreviousWordPosition(c.DeepEquivalent()));
}
Position StartOfWordPosition(const VisiblePosition& position, EWordSide side) {
......
......@@ -451,7 +451,7 @@ TEST_P(ParameterizedVisibleUnitsWordTest, NextWordSkipTab) {
//----
TEST_F(VisibleUnitsWordTest, PreviousWordBasic) {
TEST_P(ParameterizedVisibleUnitsWordTest, PreviousWordBasic) {
EXPECT_EQ("<p> |(1) abc def</p>", DoPreviousWord("<p>| (1) abc def</p>"));
EXPECT_EQ("<p> |(1) abc def</p>", DoPreviousWord("<p> |(1) abc def</p>"));
EXPECT_EQ("<p> |(1) abc def</p>", DoPreviousWord("<p> (|1) abc def</p>"));
......
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