Revert of Implement eliding/truncating at end in RenderText

ui/gfx/render_text_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.
 
 #include "ui/gfx/render_text.h"
 
 #include <algorithm>
 
 #include "base/format_macros.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 383 matching lines @@
   EXPECT_EQ(valid_expect_0_and_1, render_text->GetLayoutText());
   render_text->RenderText::SetObscuredRevealIndex(2);
   EXPECT_EQ(ASCIIToUTF16("*h***"), render_text->GetLayoutText());
   render_text->RenderText::SetObscuredRevealIndex(5);
   const char16 valid_expect_5_and_6[] = {'*', '*', '*', '*', 0xD800, 0xDC00, 0};
   EXPECT_EQ(valid_expect_5_and_6, render_text->GetLayoutText());
   render_text->RenderText::SetObscuredRevealIndex(6);
   EXPECT_EQ(valid_expect_5_and_6, render_text->GetLayoutText());
 }
 
-TEST_F(RenderTextTest, ElidedText) {
-  // TODO(skanuj) : Add more test cases for following
-  // - RenderText styles.
-  // - Cross interaction of truncate, elide and obscure.
-  // - ElideText tests from text_elider.cc.
-  struct {
-    const wchar_t* text;
-    const wchar_t* layout_text;
-  } cases[] = {
-    // Strings shorter than the elision width should be laid out in full.
-    { L""        , L""        },
-    { kWeak      , kWeak      },
-    { kLtr       , kLtr       },
-    { kLtrRtl    , kLtrRtl    },
-    { kLtrRtlLtr , kLtrRtlLtr },
-    { kRtl       , kRtl       },
-    { kRtlLtr    , kRtlLtr    },
-    { kRtlLtrRtl , kRtlLtrRtl },
-    // Strings as long as the elision width should be laid out in full.
-    { L"012ab",   L"012ab"   },
-    // Long strings should be elided with an ellipsis appended at the end.
-    { L"012" L"abc"                  , L"012a\x2026"               },

[msw, 2013/12/13 20:46:34]

nit: When you re-land, you can remove all the prefixes like "012" that were
copied from the TruncatedText test, where they were used to force certain
characters to be at the truncation length of index 5.

[Anuj, 2013/12/13 21:05:59]

Why does that matter? 012 is just some random string here.

[msw, 2013/12/13 21:17:10]

It's just a nit, but I asked to keep the test simple and directed. You could
prepend 50 more random characters before each of these strings, but why would
you? The less redundant code and meaningless data we have, the less future
maintainers will have to parse to understand our intent.

-    { L"012" L"ab" L"\x5d0\x5d1"     , L"012a\x2026"               },
-    { L"012" L"a" L"\x5d1" L"b"      , L"012a\x2026"               },
-    { L"01" L"\x5d0\x5d1\x5d2" L"ab" , L"01\x5d0\x5d1\x2026"       },
-    { L"ab" L"\x5d0\x5d1\x5d2" L"ab" , L"ab\x5d0\x5d1\x2026\x200f" },
-    // Surrogate pairs should be elided reasonably enough.
-    { L"0123\x0915\x093f" L"abcd"          , L"0123\x0915\x2026"          },

[msw, 2013/12/13 20:46:34]

Hmm, I missed this in the original review. In the TruncatedText test, the whole
surrogate pair is truncated. \x0915 is devangari 'क', and \x093f is 'ि', which
together is "कि", though the order might be wrong... Anyway, I'm not sure the
string "0123िक" should be elided to "0123क…", but a native speaker would know
better; I suggest filing a bug to follow up and adding a comment here when you
re-land.

[Anuj, 2013/12/13 21:05:59]

I am a native speaker of Hindi (script devanagari), and this is not the ideal
behavior. But the x0915 (devangari 'क'), and \x093f ('ि') do not make surrogate
pairs. This is a matter of complex script where the trailing character gets
rendered visually before the preceding character. The surrogate pairs are about
the unicode codepoints which need two UTF-16 characters.

I think handling eliding with complex script needs lot more work, and I think
RoI is not justified.

I will document this behavior.

-    { L"0\x05e9\x05bc\x05c1\x05b8"         , L"0\x05e9\x05bc\x05c1\x05b8" },
-    { L"0\x05e9\x05bc\x05c1\x05b8" L"ab"   , L"0\x05e9\x05bc\x2026"       },
-    { L"01\x05e9\x05bc\x05c1\x05b8" L"ab"  , L"01\x05e9\x2026"            },
-    { L"012\x05e9\x05bc\x05c1\x05b8" L"ab" , L"012\x2026"                 },
-    { L"012\xF0\x9D\x84\x9E"               , L"012\xF0\x2026"             },

[msw, 2013/12/13 20:46:34]

Oddly, I don't remember why this failing test case uses the UTF8
"\xF0\x9D\x84\x9E" for MUSICAL SYMBOL G CLEF when it seems like it should use
the UTF-16 "\xD834\xDD1E", and both code points should probably be elided and
truncated in this test and in TruncatedText. I'm not sure why we'd expect 0xf0
to be left in either string... Can you try fixing up both cases when you
re-land?
Still, this test case may fail on WinXP with those fixes, even if it passes on
other windows versions, perhaps due to a font issue. If you want to be cautious,
you can limit this case to non-windows or windows where base::win::GetVersion()
>= base::win::VERSION_VISTA with a TODO comment and perhaps a bug link, and then
try to enable it everywhere in a follow-up CL.

[Anuj, 2013/12/13 21:05:59]

I think I will be cautious :)

-  };
-
-  scoped_ptr<RenderText> expected_render_text(RenderText::CreateInstance());
-  expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  expected_render_text->SetDisplayRect(gfx::Rect(0, 0, 9999, 100));
-
-  scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
-  render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  render_text->SetElideBehavior(gfx::ELIDE_AT_END);
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
-    expected_render_text->SetText(WideToUTF16(cases[i].layout_text));
-    render_text->SetDisplayRect(
-        gfx::Rect(0, 0, expected_render_text->GetContentWidth(), 100));
-    render_text->SetText(WideToUTF16(cases[i].text));
-    EXPECT_EQ(WideToUTF16(cases[i].text), render_text->text());
-    EXPECT_EQ(WideToUTF16(cases[i].layout_text), render_text->GetLayoutText())
-        << "->For case " << i << ": " << cases[i].text << "\n";
-    expected_render_text->SetText(base::string16());
-  }
-}
-
-TEST_F(RenderTextTest, ElidedObscuredText) {
-  scoped_ptr<RenderText> expected_render_text(RenderText::CreateInstance());
-  expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  expected_render_text->SetDisplayRect(gfx::Rect(0, 0, 9999, 100));
-  expected_render_text->SetText(WideToUTF16(L"**\x2026"));
-
-  scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
-  render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  render_text->SetElideBehavior(gfx::ELIDE_AT_END);
-  render_text->SetDisplayRect(
-      gfx::Rect(0, 0, expected_render_text->GetContentWidth(), 100));
-  render_text->SetObscured(true);
-  render_text->SetText(WideToUTF16(L"abcdef"));
-  EXPECT_EQ(WideToUTF16(L"abcdef"), render_text->text());
-  EXPECT_EQ(WideToUTF16(L"**\x2026"), render_text->GetLayoutText());
-}
-
 TEST_F(RenderTextTest, TruncatedText) {
   struct {
     const wchar_t* text;
     const wchar_t* layout_text;
   } cases[] = {
     // Strings shorter than the truncation length should be laid out in full.
     { L"",        L""        },
     { kWeak,      kWeak      },
     { kLtr,       kLtr       },
     { kLtrRtl,    kLtrRtl    },
@@ skipping 1384 matching lines @@
   render_text->SetText(WideToUTF16(L"x \x25B6 y"));
   render_text->EnsureLayout();
   ASSERT_EQ(3U, render_text->runs_.size());
   EXPECT_EQ(Range(0, 2), render_text->runs_[0]->range);
   EXPECT_EQ(Range(2, 3), render_text->runs_[1]->range);
   EXPECT_EQ(Range(3, 5), render_text->runs_[2]->range);
 }
 #endif  // defined(OS_WIN)
 
 }  // namespace gfx