Commit 37f9a307 authored by rlanday's avatar rlanday Committed by Commit Bot

Change behavior of SpellChecker::GetSpellCheckMarkerTouchingSelection()

When I added this method, I had added some extra logic for the case where we
have a collapsed selection to make it find spell check markers the caret is
touching an endpoint of. This was to support the spell check menu on Android,
where tapping the endpoint of a word brings up the menu (on other platforms you
need to right-click in the interior of the word for spell check options to
appear in the context menu).

Upon further reflection, we decided we should put this logic somewhere else.
We're going to keep this method around though because we're going to make use of
it in ContextMenuClient::SelectMisspellingAsync().

I'm renaming this method to GetSpellCheckMarkerUnderSelection() to try to make
the name less misleading.

BUG=736181

Review-Url: https://codereview.chromium.org/2954763002
Cr-Commit-Position: refs/heads/master@{#488476}
parent 4d1f330a
......@@ -95,45 +95,6 @@ SelectionInDOMTree SelectWord(const VisiblePosition& position) {
.Build();
}
EphemeralRange ExpandSelectionRangeIfNecessary(
const VisibleSelection& selection) {
DCHECK(!selection.IsNone());
// If some text is actually selected, we can use the selection range as-is to
// check for a marker. If no text is selected (we just have a caret
// somewhere), we need to expand one character on either side so we can find
// a spelling marker immediately before or after the caret.
// (The spelling markers on these words may be anchored to a different node
// than the collapsed selection's Position is, but once we expand the
// selection, if we're next to a marker, either the start or end point should
// now be anchored relative to the same text node as that marker.)
// Some text is actually selected
if (selection.IsRange())
return EphemeralRange(selection.Start(), selection.End());
// No text is actually selected, need to expand the selection range. This is
// to make sure we can find a marker touching the caret. E.g. if we have:
// <span>word1 <b>word2</b></span>
// with a caret selection immediately before "word2", there's one text node
// immediately before the caret ("word1 ") and one immediately after
// ("word2"). Expanding the selection by one character on either side ensures
// we get a range that intersects both neighboring text nodes (if they exist).
const VisiblePosition& caret_position = selection.VisibleStart();
const Position& previous_position =
PreviousPositionOf(caret_position).DeepEquivalent();
const Position& next_position =
NextPositionOf(caret_position).DeepEquivalent();
return EphemeralRange(
previous_position.IsNull() ? caret_position.DeepEquivalent()
: previous_position,
next_position.IsNull() ? caret_position.DeepEquivalent() : next_position);
}
} // namespace
SpellChecker* SpellChecker::Create(LocalFrame& frame) {
......@@ -869,14 +830,13 @@ void SpellChecker::RemoveSpellingAndGrammarMarkers(const HTMLElement& element,
}
Optional<std::pair<Node*, SpellCheckMarker*>>
SpellChecker::GetSpellCheckMarkerTouchingSelection() {
SpellChecker::GetSpellCheckMarkerUnderSelection() {
const VisibleSelection& selection =
GetFrame().Selection().ComputeVisibleSelectionInDOMTree();
if (selection.IsNone())
return Optional<std::pair<Node*, SpellCheckMarker*>>();
const EphemeralRange& range_to_check =
ExpandSelectionRangeIfNecessary(selection);
const EphemeralRange& range_to_check = FirstEphemeralRangeOf(selection);
Node* const start_container =
range_to_check.StartPosition().ComputeContainerNode();
......@@ -907,7 +867,7 @@ SpellChecker::GetSpellCheckMarkerTouchingSelection() {
void SpellChecker::ReplaceMisspelledRange(const String& text) {
const Optional<std::pair<Node*, SpellCheckMarker*>>& node_and_marker =
GetSpellCheckMarkerTouchingSelection();
GetSpellCheckMarkerUnderSelection();
if (!node_and_marker)
return;
......
......@@ -76,7 +76,7 @@ class CORE_EXPORT SpellChecker final : public GarbageCollected<SpellChecker> {
void RespondToChangedSelection(const Position& old_selection_start,
TypingContinuation);
Optional<std::pair<Node*, SpellCheckMarker*>>
GetSpellCheckMarkerTouchingSelection();
GetSpellCheckMarkerUnderSelection();
void ReplaceMisspelledRange(const String&);
void RemoveSpellingMarkers();
void RemoveSpellingMarkersUnderWords(const Vector<String>& words);
......
......@@ -128,8 +128,7 @@ TEST_F(SpellCheckerTest, MarkAndReplaceForHandlesMultipleReplacements) {
ToSpellCheckMarker(GetDocument().Markers().Markers()[0])->Description());
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_FirstCharSelected) {
TEST_F(SpellCheckerTest, GetSpellCheckMarkerUnderSelection_FirstCharSelected) {
SetBodyContent(
"<div contenteditable>"
"spllchck"
......@@ -149,12 +148,11 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
.GetSpellCheckMarkerUnderSelection();
EXPECT_TRUE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_LastCharSelected) {
TEST_F(SpellCheckerTest, GetSpellCheckMarkerUnderSelection_LastCharSelected) {
SetBodyContent(
"<div contenteditable>"
"spllchck"
......@@ -174,12 +172,12 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
.GetSpellCheckMarkerUnderSelection();
EXPECT_TRUE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_SingleCharWordSelected) {
GetSpellCheckMarkerUnderSelection_SingleCharWordSelected) {
SetBodyContent(
"<div contenteditable>"
"s"
......@@ -199,12 +197,12 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
.GetSpellCheckMarkerUnderSelection();
EXPECT_TRUE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_CaretLeftOfSingleCharWord) {
GetSpellCheckMarkerUnderSelection_CaretLeftOfSingleCharWord) {
SetBodyContent(
"<div contenteditable>"
"s"
......@@ -224,12 +222,12 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
EXPECT_TRUE(result);
.GetSpellCheckMarkerUnderSelection();
EXPECT_FALSE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_CaretRightOfSingleCharWord) {
GetSpellCheckMarkerUnderSelection_CaretRightOfSingleCharWord) {
SetBodyContent(
"<div contenteditable>"
"s"
......@@ -249,12 +247,12 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
EXPECT_TRUE(result);
.GetSpellCheckMarkerUnderSelection();
EXPECT_FALSE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_CaretLeftOfMultiCharWord) {
GetSpellCheckMarkerUnderSelection_CaretLeftOfMultiCharWord) {
SetBodyContent(
"<div contenteditable>"
"spllchck"
......@@ -274,12 +272,12 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
EXPECT_TRUE(result);
.GetSpellCheckMarkerUnderSelection();
EXPECT_FALSE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_CaretRightOfMultiCharWord) {
GetSpellCheckMarkerUnderSelection_CaretRightOfMultiCharWord) {
SetBodyContent(
"<div contenteditable>"
"spllchck"
......@@ -299,12 +297,11 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
EXPECT_TRUE(result);
.GetSpellCheckMarkerUnderSelection();
EXPECT_FALSE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_CaretMiddleOfWord) {
TEST_F(SpellCheckerTest, GetSpellCheckMarkerUnderSelection_CaretMiddleOfWord) {
SetBodyContent(
"<div contenteditable>"
"spllchck"
......@@ -324,12 +321,12 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
.GetSpellCheckMarkerUnderSelection();
EXPECT_TRUE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_CaretOneCharLeftOfMisspelling) {
GetSpellCheckMarkerUnderSelection_CaretOneCharLeftOfMisspelling) {
SetBodyContent(
"<div contenteditable>"
"a spllchck"
......@@ -349,12 +346,12 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
.GetSpellCheckMarkerUnderSelection();
EXPECT_FALSE(result);
}
TEST_F(SpellCheckerTest,
GetSpellCheckMarkerTouchingSelection_CaretOneCharRightOfMisspelling) {
GetSpellCheckMarkerUnderSelection_CaretOneCharRightOfMisspelling) {
SetBodyContent(
"<div contenteditable>"
"spllchck a"
......@@ -374,7 +371,7 @@ TEST_F(SpellCheckerTest,
GetDocument()
.GetFrame()
->GetSpellChecker()
.GetSpellCheckMarkerTouchingSelection();
.GetSpellCheckMarkerUnderSelection();
EXPECT_FALSE(result);
}
......
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