Improve text elision in TruncateString().

ui/gfx/text_elider_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Unit tests for eliding and formatting utility functions.
 
 #include "ui/gfx/text_elider.h"
 
 #include <vector>
 
@@ skipping 1052 matching lines @@
   EXPECT_TRUE(ElideRectangleString(str, 3, 3, true, &output));
   EXPECT_EQ(out, output);
 }
 
 // TODO(338784): Enable this on android.
 #if defined(OS_ANDROID)
 #define MAYBE_TruncateString DISABLED_TruncateString
 #else
 #define MAYBE_TruncateString TruncateString
 #endif
 TEST(TextEliderTest, MAYBE_TruncateString) {

[msw, 2015/09/17 18:42:44]

Adding a test with surrogate pairs, etc. would be nice.

[Peter Kasting, 2015/09/19 02:43:25]

Since the code can get those wrong today, let me do that as part of my next
patch.  I agree we should be testing them.

[msw, 2015/09/21 18:28:10]

Acknowledged.

-  base::string16 string = ASCIIToUTF16("foooooey    bxxxar baz");
+  base::string16 str = ASCIIToUTF16("fooooey    bxxxar baz  ");
 
-  // Tests that apply to both break behaviors:
+  // Test breaking at character 0.
+  EXPECT_EQ(base::string16(), TruncateString(str, 0, WORD_BREAK));
+  EXPECT_EQ(base::string16(), TruncateString(str, 0, CHARACTER_BREAK));
 
-  // Make sure it doesn't modify the string if length > string length.
-  EXPECT_EQ(string, TruncateString(string, 100, WORD_BREAK));
-  EXPECT_EQ(string, TruncateString(string, 100, CHARACTER_BREAK));
+  // Test breaking at character 1.
+  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(str, 1, WORD_BREAK)));

[msw, 2015/09/17 18:42:44]

nit: maybe EXPECT_STREQ would make UTF16ToWide unnecessary?

[Peter Kasting, 2015/09/19 02:43:25]

Nah, that just means you need to supply two pointers, and they have to be the
same type.  The issue is that on Mac there's no way to do a raw string16 pointer
literal.

+  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(str, 1, CHARACTER_BREAK)));
 
-  // Test no characters.
-  EXPECT_EQ(L"", UTF16ToWide(TruncateString(string, 0, WORD_BREAK)));
-  EXPECT_EQ(L"", UTF16ToWide(TruncateString(string, 0, CHARACTER_BREAK)));
+  // Test breaking in the middle of the first word.
+  EXPECT_EQ(L"f\x2026", UTF16ToWide(TruncateString(str, 2, WORD_BREAK)));
+  EXPECT_EQ(L"f\x2026", UTF16ToWide(TruncateString(str, 2, CHARACTER_BREAK)));
 
-  // Test 1 character.
-  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(string, 1, WORD_BREAK)));
-  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(string, 1, CHARACTER_BREAK)));
+  // Test breaking in between words.
+  EXPECT_EQ(L"fooooey\x2026", UTF16ToWide(TruncateString(str, 9, WORD_BREAK)));
+  EXPECT_EQ(L"fooooey\x2026",
+            UTF16ToWide(TruncateString(str, 9, CHARACTER_BREAK)));
 
-  // Test completely truncates string if break is on initial whitespace.
-  EXPECT_EQ(L"\x2026",
-            UTF16ToWide(TruncateString(ASCIIToUTF16("   "), 2, WORD_BREAK)));
-  EXPECT_EQ(L"\x2026",
-            UTF16ToWide(TruncateString(ASCIIToUTF16("   "), 2,
-                                       CHARACTER_BREAK)));
+  // Test breaking at the start of a later word.

[msw, 2015/09/17 18:42:44]

It's probably worthwhile to test index 12 too (but maybe that's mostly covered
by testing index 5 below).

[Peter Kasting, 2015/09/19 02:43:25]

Done.  Expanded the tests below by a case as well.

+  EXPECT_EQ(L"fooooey\x2026", UTF16ToWide(TruncateString(str, 11, WORD_BREAK)));
+  EXPECT_EQ(L"fooooey\x2026",
+            UTF16ToWide(TruncateString(str, 11, CHARACTER_BREAK)));
 
-  // Break-only-at-word-boundaries tests:
+  // Test breaking in the middle of a word.
+  EXPECT_EQ(L"fooooey\x2026", UTF16ToWide(TruncateString(str, 14, WORD_BREAK)));
+  EXPECT_EQ(L"fooooey    bx\x2026",
+            UTF16ToWide(TruncateString(str, 14, CHARACTER_BREAK)));
 
-  // Test adds ... at right spot when there is enough room to break at a
-  // word boundary.
-  EXPECT_EQ(L"foooooey\x2026", UTF16ToWide(TruncateString(string, 14,
-                                                          WORD_BREAK)));
+  // Test breaking in whitespace at the end of the string.
+  EXPECT_EQ(L"fooooey    bxxxar baz\x2026",
+            UTF16ToWide(TruncateString(str, 22, WORD_BREAK)));
+  EXPECT_EQ(L"fooooey    bxxxar baz\x2026",
+            UTF16ToWide(TruncateString(str, 22, CHARACTER_BREAK)));
 
-  // Test adds ... at right spot when there is not enough space in first word.
-  EXPECT_EQ(L"f\x2026", UTF16ToWide(TruncateString(string, 2, WORD_BREAK)));
+  // Test breaking at the end of the string.
+  EXPECT_EQ(str, TruncateString(str, str.length(), WORD_BREAK));
+  EXPECT_EQ(str, TruncateString(str, str.length(), CHARACTER_BREAK));
 
-  // Test adds ... at right spot when there is not enough room to break at a
-  // word boundary.
-  EXPECT_EQ(L"foooooey\x2026", UTF16ToWide(TruncateString(string, 11,
-                                                          WORD_BREAK)));
+  // Test breaking past the end of the string.
+  EXPECT_EQ(str, TruncateString(str, str.length() + 10, WORD_BREAK));
+  EXPECT_EQ(str, TruncateString(str, str.length() + 10, CHARACTER_BREAK));
 
-  // Break-anywhere tests:
 
-  // Test adds ... at right spot within a word.
-  EXPECT_EQ(L"f\x2026", UTF16ToWide(TruncateString(string, 2,
-                                                   CHARACTER_BREAK)));
+  // Tests of strings with leading whitespace:
+  base::string16 str2 = ASCIIToUTF16("   foo");
 
-  // Test removes trailing whitespace if break falls between words.
-  EXPECT_EQ(L"foooooey\x2026", UTF16ToWide(TruncateString(string, 12,
-                                                          CHARACTER_BREAK)));
+  // Test breaking in leading whitespace.
+  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(str2, 2, WORD_BREAK)));
+  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(str2, 2, CHARACTER_BREAK)));
+
+  // Test breaking at the beginning of the first word, with leading whitespace.
+  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(str2, 4, WORD_BREAK)));
+  EXPECT_EQ(L"\x2026", UTF16ToWide(TruncateString(str2, 4, CHARACTER_BREAK)));
+
+  // Test breaking in the middle of the first word, with leading whitespace.
+  EXPECT_EQ(L"   f\x2026", UTF16ToWide(TruncateString(str2, 5, WORD_BREAK)));
+  EXPECT_EQ(L"   f\x2026",
+            UTF16ToWide(TruncateString(str2, 5, CHARACTER_BREAK)));
 }
 
 }  // namespace gfx