Commit e2db13ac authored by My Nguyen's avatar My Nguyen Committed by Commit Bot

Get correct offset for ime_text_spans

Autocorrect range and bounds are wrong after new line because the offset
is in a different node. Need to use PlainTextRange instead.

Bug: 1114543
Change-Id: I913bd8a2d150388fc034236eba42e5983f765383
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2344566
Commit-Queue: My Nguyen <myy@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796755}
parent 1cad29d8
...@@ -1810,23 +1810,32 @@ WebVector<ui::ImeTextSpan> InputMethodController::GetImeTextSpansAroundPosition( ...@@ -1810,23 +1810,32 @@ WebVector<ui::ImeTextSpan> InputMethodController::GetImeTextSpansAroundPosition(
// Only queries Suggestion markers for now. // Only queries Suggestion markers for now.
// This can be expanded when browser needs information for // This can be expanded when browser needs information for
// other types of markers. // other types of markers.
const DocumentMarkerVector& marker_list = const HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>&
GetDocument().Markers().MarkersAroundPosition( node_marker_pairs = GetDocument().Markers().MarkersAroundPosition(
ToPositionInFlatTree(range.StartPosition()), ToPositionInFlatTree(range.StartPosition()),
DocumentMarker::MarkerTypes::Suggestion()); DocumentMarker::MarkerTypes::Suggestion());
for (DocumentMarker* marker : marker_list) { for (const std::pair<Member<const Text>, Member<DocumentMarker>>&
if (marker->GetType() == DocumentMarker::MarkerType::kSuggestion) { node_marker_pair : node_marker_pairs) {
ImeTextSpan::Type type = ConvertSuggestionMarkerType( SuggestionMarker* marker =
To<SuggestionMarker>(marker)->GetSuggestionType()); To<SuggestionMarker>(node_marker_pair.second.Get());
if (ShouldGetImeTextSpansAroundPosition(type)) { ImeTextSpan::Type type =
ime_text_spans.emplace_back( ConvertSuggestionMarkerType(marker->GetSuggestionType());
ImeTextSpan(type, marker->StartOffset(), marker->EndOffset(), if (ShouldGetImeTextSpansAroundPosition(type)) {
Color::kTransparent, ImeTextSpanThickness::kNone, const Text* node = node_marker_pair.first;
ImeTextSpanUnderlineStyle::kNone, Color::kTransparent, const EphemeralRange& marker_ephemeral_range =
Color::kTransparent) EphemeralRange(Position(node, marker->StartOffset()),
.ToUiImeTextSpan()); Position(node, marker->EndOffset()));
} PlainTextRange marker_plain_text_range =
PlainTextRangeForEphemeralRange(marker_ephemeral_range).second;
ime_text_spans.emplace_back(
ImeTextSpan(type, marker_plain_text_range.Start(),
marker_plain_text_range.End(), Color::kTransparent,
ImeTextSpanThickness::kNone,
ImeTextSpanUnderlineStyle::kNone, Color::kTransparent,
Color::kTransparent)
.ToUiImeTextSpan());
} }
} }
......
...@@ -540,15 +540,18 @@ DocumentMarker* DocumentMarkerController::FirstMarkerIntersectingOffsetRange( ...@@ -540,15 +540,18 @@ DocumentMarker* DocumentMarkerController::FirstMarkerIntersectingOffsetRange(
return nullptr; return nullptr;
} }
DocumentMarkerVector DocumentMarkerController::MarkersAroundPosition( HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>
DocumentMarkerController::MarkersAroundPosition(
const PositionInFlatTree& position, const PositionInFlatTree& position,
DocumentMarker::MarkerTypes types) { DocumentMarker::MarkerTypes types) {
DocumentMarkerVector result; HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>
node_marker_pairs;
if (position.IsNull()) if (position.IsNull())
return result; return node_marker_pairs;
if (!PossiblyHasMarkers(types)) if (!PossiblyHasMarkers(types))
return result; return node_marker_pairs;
const PositionInFlatTree& start = SearchAroundPositionStart(position); const PositionInFlatTree& start = SearchAroundPositionStart(position);
const PositionInFlatTree& end = SearchAroundPositionEnd(position); const PositionInFlatTree& end = SearchAroundPositionEnd(position);
...@@ -556,7 +559,7 @@ DocumentMarkerVector DocumentMarkerController::MarkersAroundPosition( ...@@ -556,7 +559,7 @@ DocumentMarkerVector DocumentMarkerController::MarkersAroundPosition(
if (start > end) { if (start > end) {
// TODO(crbug/1114021): Investigate why this might happen. // TODO(crbug/1114021): Investigate why this might happen.
NOTREACHED() << "|start| should be before |end|."; NOTREACHED() << "|start| should be before |end|.";
return result; return node_marker_pairs;
} }
const Node* const start_node = start.ComputeContainerNode(); const Node* const start_node = start.ComputeContainerNode();
...@@ -590,11 +593,14 @@ DocumentMarkerVector DocumentMarkerController::MarkersAroundPosition( ...@@ -590,11 +593,14 @@ DocumentMarkerVector DocumentMarkerController::MarkersAroundPosition(
if (!list) if (!list)
continue; continue;
result.AppendVector( const DocumentMarkerVector& markers =
list->MarkersIntersectingRange(start_range_offset, end_range_offset)); list->MarkersIntersectingRange(start_range_offset, end_range_offset);
for (DocumentMarker* marker : markers)
node_marker_pairs.push_back(std::make_pair(&To<Text>(node), marker));
} }
} }
return result; return node_marker_pairs;
} }
HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>> HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>
......
...@@ -143,10 +143,11 @@ class CORE_EXPORT DocumentMarkerController final ...@@ -143,10 +143,11 @@ class CORE_EXPORT DocumentMarkerController final
// the specified type. If the position is neither at the boundary or inside a // the specified type. If the position is neither at the boundary or inside a
// word, expands the position to cover the space between the end of the // word, expands the position to cover the space between the end of the
// previous and the start of the next words. If such markers exist, this // previous and the start of the next words. If such markers exist, this
// method will return all of them. Otherwise, this method will return an empty // method will return all of them in their corresponding node. Otherwise,
// list. // this method will return an empty list.
DocumentMarkerVector MarkersAroundPosition(const PositionInFlatTree& position, HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>
DocumentMarker::MarkerTypes types); MarkersAroundPosition(const PositionInFlatTree& position,
DocumentMarker::MarkerTypes types);
// Return all markers of the specified types whose interiors have non-empty // Return all markers of the specified types whose interiors have non-empty
// overlap with the specified range. Note that the range can be collapsed, in // overlap with the specified range. Note that the range can be collapsed, in
// in which case markers containing the position in their interiors are // in which case markers containing the position in their interiors are
......
...@@ -522,31 +522,37 @@ TEST_F(DocumentMarkerControllerTest, MarkersAroundPosition) { ...@@ -522,31 +522,37 @@ TEST_F(DocumentMarkerControllerTest, MarkersAroundPosition) {
EphemeralRange(Position(text, 4), Position(text, 7))); EphemeralRange(Position(text, 4), Position(text, 7)));
// Query for spellcheck markers at the start of "123". // Query for spellcheck markers at the start of "123".
const DocumentMarkerVector& list1 = MarkerController().MarkersAroundPosition( const HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>&
PositionInFlatTree(text, 0), DocumentMarker::MarkerTypes::Misspelling()); result1 = MarkerController().MarkersAroundPosition(
PositionInFlatTree(text, 0),
DocumentMarker::MarkerTypes::Misspelling());
EXPECT_EQ(1u, list1.size()); EXPECT_EQ(1u, result1.size());
EXPECT_EQ(DocumentMarker::kSpelling, list1[0]->GetType()); EXPECT_EQ(DocumentMarker::kSpelling, result1[0].second->GetType());
EXPECT_EQ(0u, list1[0]->StartOffset()); EXPECT_EQ(0u, result1[0].second->StartOffset());
EXPECT_EQ(3u, list1[0]->EndOffset()); EXPECT_EQ(3u, result1[0].second->EndOffset());
// Query for spellcheck markers in the middle of "123". // Query for spellcheck markers in the middle of "123".
const DocumentMarkerVector& list2 = MarkerController().MarkersAroundPosition( const HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>&
PositionInFlatTree(text, 3), DocumentMarker::MarkerTypes::Misspelling()); result2 = MarkerController().MarkersAroundPosition(
PositionInFlatTree(text, 3),
DocumentMarker::MarkerTypes::Misspelling());
EXPECT_EQ(1u, list2.size()); EXPECT_EQ(1u, result2.size());
EXPECT_EQ(DocumentMarker::kSpelling, list2[0]->GetType()); EXPECT_EQ(DocumentMarker::kSpelling, result2[0].second->GetType());
EXPECT_EQ(0u, list2[0]->StartOffset()); EXPECT_EQ(0u, result2[0].second->StartOffset());
EXPECT_EQ(3u, list2[0]->EndOffset()); EXPECT_EQ(3u, result2[0].second->EndOffset());
// Query for spellcheck markers at the end of "123". // Query for spellcheck markers at the end of "123".
const DocumentMarkerVector& list3 = MarkerController().MarkersAroundPosition( const HeapVector<std::pair<Member<const Text>, Member<DocumentMarker>>>&
PositionInFlatTree(text, 3), DocumentMarker::MarkerTypes::Misspelling()); result3 = MarkerController().MarkersAroundPosition(
PositionInFlatTree(text, 3),
DocumentMarker::MarkerTypes::Misspelling());
EXPECT_EQ(1u, list3.size()); EXPECT_EQ(1u, result3.size());
EXPECT_EQ(DocumentMarker::kSpelling, list3[0]->GetType()); EXPECT_EQ(DocumentMarker::kSpelling, result3[0].second->GetType());
EXPECT_EQ(0u, list3[0]->StartOffset()); EXPECT_EQ(0u, result3[0].second->StartOffset());
EXPECT_EQ(3u, list3[0]->EndOffset()); EXPECT_EQ(3u, result3[0].second->EndOffset());
} }
TEST_F(DocumentMarkerControllerTest, MarkersIntersectingRange) { TEST_F(DocumentMarkerControllerTest, MarkersIntersectingRange) {
......
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