Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(890)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp

Issue 2755013004: Improve how DocumentMarkerController updates markers in response to text edits (Closed)
Patch Set: Rebase (Vector::remove() => Vector::erase()) Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h » ('j') | third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
diff --git a/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp b/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
index 841eb7700b577d213aad53410443c8a00b8bb7ae..e151c8f0d1428c9c314cf1b622bcf97d34abf7aa 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
@@ -1542,10 +1542,6 @@ TEST_F(InputMethodControllerTest,
getMarkedText(document().markers(), div->firstChild(), 0).utf8().data());
}
-// TODO(rlanday): The behavior tested in the following DocumentMarker tests is
-// going to be changed so markers are not split when text they contain is
-// deleted
-
TEST_F(InputMethodControllerTest,
Marker_WhitespaceFixupAroundMarkerBeginningWithSpace) {
Element* div = insertHTMLElement(
@@ -1566,15 +1562,12 @@ TEST_F(InputMethodControllerTest,
controller().setCompositionFromExistingText(emptyUnderlines, 6, 10);
controller().commitText(String(""), emptyUnderlines, 0);
- // Check that the marker was split when the space at the beginning was
- // converted to an nbsp
- EXPECT_EQ(2u, document().markers().markers().size());
- EXPECT_STREQ(
- "\xC2\xA0", // UTF-8 for an nbsp
+ // Check that the marker is still attached to " text" and includes the space
+ // before "text" but not the space after
+ EXPECT_EQ(1u, document().markers().markers().size());
+ ASSERT_STREQ(
+ "\xC2\xA0text",
getMarkedText(document().markers(), div->firstChild(), 0).utf8().data());
- EXPECT_STREQ(
- "text",
- getMarkedText(document().markers(), div->firstChild(), 1).utf8().data());
}
TEST_F(InputMethodControllerTest,
@@ -1597,15 +1590,12 @@ TEST_F(InputMethodControllerTest,
controller().setCompositionFromExistingText(emptyUnderlines, 6, 10);
controller().commitText(String(""), emptyUnderlines, 0);
- // Check that the marker was split when the space at the end was
- // converted to an nbsp
- EXPECT_EQ(2u, document().markers().markers().size());
- EXPECT_STREQ(
- "text",
+ // Check that the marker is still attached to "text " and includes the space
+ // after "text" but not the space before
+ EXPECT_EQ(1u, document().markers().markers().size());
+ ASSERT_STREQ(
+ "text\xC2\xA0",
getMarkedText(document().markers(), div->firstChild(), 0).utf8().data());
- EXPECT_STREQ(
- "\xC2\xA0", // UTF-8 for an nbsp
- getMarkedText(document().markers(), div->firstChild(), 1).utf8().data());
}
TEST_F(InputMethodControllerTest,
@@ -1629,18 +1619,12 @@ TEST_F(InputMethodControllerTest,
controller().setCompositionFromExistingText(emptyUnderlines, 6, 10);
controller().commitText(String(""), emptyUnderlines, 0);
- // Check that the marker was split into three pieces when the two spaces were
- // converted to nbsps
- EXPECT_EQ(3u, document().markers().markers().size());
- EXPECT_STREQ(
- "\xC2\xA0", // UTF-8 for an nbsp
+ // Check that the marker is still attached to " text " and includes both the
+ // space before "text" and the space after
+ EXPECT_EQ(1u, document().markers().markers().size());
+ ASSERT_STREQ(
+ "\xC2\xA0text\xC2\xA0",
getMarkedText(document().markers(), div->firstChild(), 0).utf8().data());
- EXPECT_STREQ(
- "text",
- getMarkedText(document().markers(), div->firstChild(), 1).utf8().data());
- EXPECT_STREQ(
- "\xC2\xA0", // UTF-8 for an nbsp
- getMarkedText(document().markers(), div->firstChild(), 2).utf8().data());
}
TEST_F(InputMethodControllerTest, Marker_ReplaceStartOfMarker) {
@@ -1658,11 +1642,10 @@ TEST_F(InputMethodControllerTest, Marker_ReplaceStartOfMarker) {
controller().setCompositionFromExistingText(emptyUnderlines, 0, 7);
controller().commitText(String("Original"), emptyUnderlines, 0);
- // Verify marker is under "al text"
- // ("Initial" and "Original" have "al" as a common suffix)
+ // Verify marker is under "Original text"
EXPECT_EQ(1u, document().markers().markers().size());
- EXPECT_STREQ(
- "al text",
+ ASSERT_STREQ(
+ "Original text",
getMarkedText(document().markers(), div->firstChild(), 0).utf8().data());
}
@@ -1704,10 +1687,10 @@ TEST_F(InputMethodControllerTest, Marker_ReplaceEndOfMarker) {
controller().setCompositionFromExistingText(emptyUnderlines, 8, 12);
controller().commitText(String("string"), emptyUnderlines, 0);
- // Verify marker is under "Initial "
+ // Verify marker is under "Initial string"
EXPECT_EQ(1u, document().markers().markers().size());
- EXPECT_STREQ(
- "Initial ",
+ ASSERT_STREQ(
+ "Initial string",
getMarkedText(document().markers(), div->firstChild(), 0).utf8().data());
}
@@ -1751,8 +1734,11 @@ TEST_F(InputMethodControllerTest, Marker_ReplaceEntireMarker) {
controller().setCompositionFromExistingText(emptyUnderlines, 8, 12);
controller().commitText(String("string"), emptyUnderlines, 0);
- // Verify marker was removed
- EXPECT_EQ(0u, document().markers().markers().size());
+ // Verify marker is under "string"
+ EXPECT_EQ(1u, document().markers().markers().size());
+ ASSERT_STREQ(
+ "string",
+ getMarkedText(document().markers(), div->firstChild(), 0).utf8().data());
}
TEST_F(InputMethodControllerTest, Marker_ReplaceTextWithMarkerAtBeginning) {
@@ -1797,4 +1783,137 @@ TEST_F(InputMethodControllerTest, Marker_ReplaceTextWithMarkerAtEnd) {
EXPECT_EQ(0u, document().markers().markers().size());
}
+TEST_F(InputMethodControllerTest, Marker_Deletions) {
Xiaocheng 2017/03/29 18:31:02 Could you split it into three tests?
+ Element* div = insertHTMLElement(
+ "<div id='sample' contenteditable>1111122222333334444455555</div>",
+ "sample");
+
+ EphemeralRange markerRange = PlainTextRange(0, 5).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ markerRange = PlainTextRange(5, 10).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ markerRange = PlainTextRange(10, 15).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ markerRange = PlainTextRange(15, 20).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ markerRange = PlainTextRange(20, 25).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ EXPECT_EQ(5u, document().markers().markers().size());
+
+ // Delete third marker and portions of second and fourth
+ Vector<CompositionUnderline> emptyUnderlines;
+ controller().setCompositionFromExistingText(emptyUnderlines, 8, 17);
+ controller().commitText(String(""), emptyUnderlines, 0);
+
+ // Verify markers were updated correctly
+ EXPECT_EQ(4u, document().markers().markers().size());
+
+ EXPECT_EQ(0u, document().markers().markers()[0]->startOffset());
+ EXPECT_EQ(5u, document().markers().markers()[0]->endOffset());
+
+ EXPECT_EQ(5u, document().markers().markers()[1]->startOffset());
+ EXPECT_EQ(8u, document().markers().markers()[1]->endOffset());
+
+ EXPECT_EQ(8u, document().markers().markers()[2]->startOffset());
+ EXPECT_EQ(11u, document().markers().markers()[2]->endOffset());
+
+ EXPECT_EQ(11u, document().markers().markers()[3]->startOffset());
+ EXPECT_EQ(16u, document().markers().markers()[3]->endOffset());
+
+ document().markers().clear();
+
+ markerRange = PlainTextRange(5, 10).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ // Delete exactly on a marker
+ controller().setCompositionFromExistingText(emptyUnderlines, 5, 10);
+ controller().commitText(String(""), emptyUnderlines, 0);
+ EXPECT_EQ(0u, document().markers().markers().size());
+
+ markerRange = PlainTextRange(5, 10).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ // Delete middle of marker
+ controller().setCompositionFromExistingText(emptyUnderlines, 6, 9);
+ controller().commitText(String(""), emptyUnderlines, 0);
+
+ EXPECT_EQ(2u, document().markers().markers().size());
+
+ EXPECT_EQ(5u, document().markers().markers()[0]->startOffset());
+ EXPECT_EQ(6u, document().markers().markers()[0]->endOffset());
+
+ EXPECT_EQ(6u, document().markers().markers()[1]->startOffset());
+ EXPECT_EQ(7u, document().markers().markers()[1]->endOffset());
+}
+
+TEST_F(InputMethodControllerTest, Marker_Insertions) {
Xiaocheng 2017/03/29 18:31:02 Could you split it into two tests?
+ Element* div = insertHTMLElement(
+ "<div id='sample' contenteditable>1111122222333334444455555</div>",
+ "sample");
+
+ EphemeralRange markerRange = PlainTextRange(0, 5).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ markerRange = PlainTextRange(5, 10).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ markerRange = PlainTextRange(10, 15).createRange(*div);
+ document().markers().addMarker(markerRange.startPosition(),
+ markerRange.endPosition(),
+ DocumentMarker::TextMatch);
+
+ // insert in middle of second marker
+ Vector<CompositionUnderline> emptyUnderlines;
+ controller().setComposition("", emptyUnderlines, 7, 7);
+ controller().commitText(String("66666"), emptyUnderlines, -7);
+
+ EXPECT_EQ(3u, document().markers().markers().size());
+
+ EXPECT_EQ(0u, document().markers().markers()[0]->startOffset());
+ EXPECT_EQ(5u, document().markers().markers()[0]->endOffset());
+
+ EXPECT_EQ(5u, document().markers().markers()[1]->startOffset());
+ EXPECT_EQ(15u, document().markers().markers()[1]->endOffset());
+
+ EXPECT_EQ(15u, document().markers().markers()[2]->startOffset());
+ EXPECT_EQ(20u, document().markers().markers()[2]->endOffset());
+
+ // insert between first and second markers (cursor is at position 5)
+ controller().commitText(String("77777"), emptyUnderlines, 0);
+
+ EXPECT_EQ(3u, document().markers().markers().size());
+
+ EXPECT_EQ(0u, document().markers().markers()[0]->startOffset());
+ EXPECT_EQ(5u, document().markers().markers()[0]->endOffset());
+
+ EXPECT_EQ(10u, document().markers().markers()[1]->startOffset());
+ EXPECT_EQ(20u, document().markers().markers()[1]->endOffset());
+
+ EXPECT_EQ(20u, document().markers().markers()[2]->startOffset());
+ EXPECT_EQ(25u, document().markers().markers()[2]->endOffset());
+}
+
} // namespace blink
« no previous file with comments | « no previous file | third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h » ('j') | third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp » ('J')

Powered by Google App Engine
This is Rietveld 408576698