[Mac] Text Suggestion Selection Replaces Trailing Whitespace

When the user selects a text suggestion from the
SuggestedTextTouchBar, the string returned by AppKit API has the
necessary trailing whitespace. Currently, when the current
editing word is replaced, AppKit's whitespace is added
alongside the existing trailing whitespace.

This change includes whitespace after a word or the current cursor
position into the editing word range. Then, when the editing word
is replaced, or the suggestion's text is placed at the cursor, the
existing trailing whitespace is also replaced with the suggestion's
trailing whitespace.

The EditingWordRangeTest in SuggestedTextTouchBarControllerUnitTests
was split into multiple tests. A test case was added,
WhitespaceEditingWordRangeTest, to test that
editingWordRangeFromText:cursorPosition: properly calculates the
editing word range as it moves through non-word characters. Existing
test cases were modified to reflect the new behavior with regards to
trailing whitespace characters.

Bug: 717553
Change-Id: Ie0116fc6802ede203dd2adeb691789fa1e4719f3
Reviewed-on: https://chromium-review.googlesource.com/1147335Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Commit-Queue: Tessa Nijssen <tnijssen@google.com>
Cr-Commit-Position: refs/heads/master@{#577658}

chrome/browser/ui/cocoa/touchbar/suggested_text_touch_bar_controller.mm

@@ -117,6 +117,7 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   NSCandidateListTouchBarItem* candidateListItem =
       [[NSCandidateListTouchBarItem alloc]
           initWithIdentifier:kSuggestedTextItemsTouchId];
+
   [candidateListItem setCandidates:suggestions_
                   forSelectedRange:selectionRange_
                           inString:text_];
@@ -169,6 +170,12 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   // The cursor should not be off the end of the text.
   DCHECK(cursor <= text.length());
 
+  // Default range is just the cursor position. This is used if BreakIterator
+  // cannot be initialized, there is no text in the textfield, or the cursor is
+  // at the front of a word.
+  size_t location = cursor;
+  size_t length = 0;
+
   base::i18n::BreakIterator iter(text, base::i18n::BreakIterator::BREAK_WORD);
 
   if (iter.Init()) {
@@ -178,14 +185,22 @@ class WebContentsTextObserver : public content::WebContentsObserver {
     }
 
     // If BreakIterator stopped at the end of a word, the cursor is in/at the
-    // end of a word so we return [word start, word end]
-    if (iter.IsWord())
-      return NSMakeRange(iter.prev(), iter.pos() - iter.prev());
+    // end of a word so the editing word range is [word start, word end].
+    if (iter.IsWord()) {
+      location = iter.prev();
+      length = iter.pos() - iter.prev();
+    }
+
+    // The suggestion text returned by AppKit has the necessary trailing
+    // whitespace which should replace the existing trailing whitespace.
+    // If the BreakIterator just iterated over whitespace or iterates over
+    // whitespace when it advances, modify the length of the editing word
+    // range to include the whitespace.
+    if ((iter.pos() && !iter.IsWord()) || (iter.Advance() && !iter.IsWord()))
+      length = iter.pos() - location;
   }
 
-  // Otherwise, we are not currently in a word, so we return
-  // [cursor position, cursor position]
-  return NSMakeRange(cursor, 0);
+  return NSMakeRange(location, length);
 }
 
 - (void)requestSuggestionsForText:(NSString*)text inRange:(NSRange)range {

chrome/browser/ui/cocoa/touchbar/suggested_text_touch_bar_controller_unittest.mm

@@ -17,6 +17,7 @@ const base::string16 kWord(base::ASCIIToUTF16("hello"));
 const base::string16 kWordWithTrailingWhitespace(base::ASCIIToUTF16("hello "));
 const base::string16 kWordWithLeadingWhitespace(base::ASCIIToUTF16(" hello"));
 const base::string16 kMultipleWords(base::ASCIIToUTF16("hello world"));
+const base::string16 kWhitespace(base::ASCIIToUTF16("     "));
 
 class SuggestedTextTouchBarControllerUnitTest : public CocoaTest {
  public:
@@ -48,25 +49,60 @@ TEST_F(SuggestedTextTouchBarControllerUnitTest, CollapsedCandidateListTest) {
   }
 }
 
-// Tests that the proper range representing the location of the editing word is
-// calculated for a given text and cursor position.
-TEST_F(SuggestedTextTouchBarControllerUnitTest, EditingWordRangeTest) {
-  // Test that the editing word range is simply the cursor position if there
-  // is no text.
+// Tests that the editing word range is simply the cursor position if there
+// is no text.
+TEST_F(SuggestedTextTouchBarControllerUnitTest, EmptyTextEditingWordRangeTest) {
   EXPECT_EQ(gfx::Range(0, 0), GetEditingWordRange(kEmptyText, 0));
+}
 
-  // Test that the editing word range contains the full word as the cursor
-  // moves through a word without breaks.
+// Tests that the editing word range contains the full word as the cursor
+// moves through a word without breaks.
+TEST_F(SuggestedTextTouchBarControllerUnitTest, WordEditingWordRangeTest) {
   EXPECT_EQ(gfx::Range(0, 0), GetEditingWordRange(kWord, 0));
   EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kWord, 1));
   EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kWord, 2));
   EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kWord, 3));
   EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kWord, 4));
   EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kWord, 5));
+}
+
+// Tests that the editing word range is properly calculated as the cursor moves
+// through non-word characters.
+TEST_F(SuggestedTextTouchBarControllerUnitTest,
+       WhitespaceEditingWordRangeTest) {
+  EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kWhitespace, 0));
+  EXPECT_EQ(gfx::Range(1, 5), GetEditingWordRange(kWhitespace, 1));
+  EXPECT_EQ(gfx::Range(2, 5), GetEditingWordRange(kWhitespace, 2));
+  EXPECT_EQ(gfx::Range(3, 5), GetEditingWordRange(kWhitespace, 3));
+  EXPECT_EQ(gfx::Range(4, 5), GetEditingWordRange(kWhitespace, 4));
+  EXPECT_EQ(gfx::Range(5, 5), GetEditingWordRange(kWhitespace, 5));
+}
 
-  // Tests that the editing word range changes properly as the cursor moves
-  // from non-word to word characters.
+// Tests that the editing word range changes properly as the cursor moves
+// from word to non-word characters.
+TEST_F(SuggestedTextTouchBarControllerUnitTest,
+       TrailingWhitespaceEditingWordRangeTest) {
   EXPECT_EQ(gfx::Range(0, 0),
+            GetEditingWordRange(kWordWithTrailingWhitespace, 0));
+  EXPECT_EQ(gfx::Range(0, 6),
+            GetEditingWordRange(kWordWithTrailingWhitespace, 1));
+  EXPECT_EQ(gfx::Range(0, 6),
+            GetEditingWordRange(kWordWithTrailingWhitespace, 2));
+  EXPECT_EQ(gfx::Range(0, 6),
+            GetEditingWordRange(kWordWithTrailingWhitespace, 3));
+  EXPECT_EQ(gfx::Range(0, 6),
+            GetEditingWordRange(kWordWithTrailingWhitespace, 4));
+  EXPECT_EQ(gfx::Range(0, 6),
+            GetEditingWordRange(kWordWithTrailingWhitespace, 5));
+  EXPECT_EQ(gfx::Range(6, 6),
+            GetEditingWordRange(kWordWithTrailingWhitespace, 6));
+}
+
+// Tests that the editing word range changes properly as the cursor moves
+// from non-word to word characters.
+TEST_F(SuggestedTextTouchBarControllerUnitTest,
+       LeadingWhitespaceEditingWordRangeTest) {
+  EXPECT_EQ(gfx::Range(0, 1),
             GetEditingWordRange(kWordWithLeadingWhitespace, 0));
   EXPECT_EQ(gfx::Range(1, 1),
             GetEditingWordRange(kWordWithLeadingWhitespace, 1));
@@ -80,32 +116,18 @@ TEST_F(SuggestedTextTouchBarControllerUnitTest, EditingWordRangeTest) {
             GetEditingWordRange(kWordWithLeadingWhitespace, 5));
   EXPECT_EQ(gfx::Range(1, 6),
             GetEditingWordRange(kWordWithLeadingWhitespace, 6));
+}
 
-  // Tests that the editing word range changes properly as the cursor moves
-  // from word to non-word characters.
-  EXPECT_EQ(gfx::Range(0, 0),
-            GetEditingWordRange(kWordWithTrailingWhitespace, 0));
-  EXPECT_EQ(gfx::Range(0, 5),
-            GetEditingWordRange(kWordWithTrailingWhitespace, 1));
-  EXPECT_EQ(gfx::Range(0, 5),
-            GetEditingWordRange(kWordWithTrailingWhitespace, 2));
-  EXPECT_EQ(gfx::Range(0, 5),
-            GetEditingWordRange(kWordWithTrailingWhitespace, 3));
-  EXPECT_EQ(gfx::Range(0, 5),
-            GetEditingWordRange(kWordWithTrailingWhitespace, 4));
-  EXPECT_EQ(gfx::Range(0, 5),
-            GetEditingWordRange(kWordWithTrailingWhitespace, 5));
-  EXPECT_EQ(gfx::Range(6, 6),
-            GetEditingWordRange(kWordWithTrailingWhitespace, 6));
-
-  // Tests that the editing word range changes properly as the cursor moves
-  // from word to non-word and back to word characters.
+// Tests that the editing word range is properly calculated as the cursor moves
+// from word to non-word and back to word characters.
+TEST_F(SuggestedTextTouchBarControllerUnitTest,
+       MultipleWordsEditingWordRangeTest) {
   EXPECT_EQ(gfx::Range(0, 0), GetEditingWordRange(kMultipleWords, 0));
-  EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kMultipleWords, 1));
-  EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kMultipleWords, 2));
-  EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kMultipleWords, 3));
-  EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kMultipleWords, 4));
-  EXPECT_EQ(gfx::Range(0, 5), GetEditingWordRange(kMultipleWords, 5));
+  EXPECT_EQ(gfx::Range(0, 6), GetEditingWordRange(kMultipleWords, 1));
+  EXPECT_EQ(gfx::Range(0, 6), GetEditingWordRange(kMultipleWords, 2));
+  EXPECT_EQ(gfx::Range(0, 6), GetEditingWordRange(kMultipleWords, 3));
+  EXPECT_EQ(gfx::Range(0, 6), GetEditingWordRange(kMultipleWords, 4));
+  EXPECT_EQ(gfx::Range(0, 6), GetEditingWordRange(kMultipleWords, 5));
   EXPECT_EQ(gfx::Range(6, 6), GetEditingWordRange(kMultipleWords, 6));
   EXPECT_EQ(gfx::Range(6, 11), GetEditingWordRange(kMultipleWords, 7));
   EXPECT_EQ(gfx::Range(6, 11), GetEditingWordRange(kMultipleWords, 8));