Revert of Implement eliding/truncating at end in RenderText

Revert of https://codereview.chromium.org/112063003/
Reason for revert: RenderTextTest.ElidedText fails on XP Tests (3)

TBR=msw@chromium.org,pkasting@chromium.org,jshin@chromium.org,skanuj@chromium.org
NOTREECHECKS=true
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=240694

[szym, 2013-12-13 17:59:12]

Created Revert of Implement eliding/truncating at end in RenderText

[szym, 2013-12-13 18:00:20]

On 2013/12/13 17:59:12, szym wrote:
> Created Revert of Implement eliding/truncating at end in RenderText

Failure log:

RenderTextTest.ElidedText: 
gfx\render_text_unittest.cc(452): error: Value of: render_text->GetLayoutText()
Actual: L"012\xF0\x9D\x84\x9E"
Expected: WideToUTF16(cases[i].layout_text)
Which is: L"012\xF0\x2026"
->For case 19: 012𝄞

[commit-bot: I haz the power, 2013-12-13 18:01:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szym@chromium.org/103493003/1

[commit-bot: I haz the power, 2013-12-13 18:03:16]

Change committed as 240694

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

LGTM, thanks for the revert. I posted some comments that should help Anuj
re-land with more correct tests.

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest.cc
File ui/gfx/render_text_unittest.cc (left):

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest....
ui/gfx/render_text_unittest.cc:425: { L"012" L"abc"                  ,
L"012a\x2026"               },
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.

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest....
ui/gfx/render_text_unittest.cc:431: { L"0123\x0915\x093f" L"abcd"          ,
L"0123\x0915\x2026"          },
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.

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest....
ui/gfx/render_text_unittest.cc:436: { L"012\xF0\x9D\x84\x9E"               ,
L"012\xF0\x2026"             },
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:58]

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest.cc
File ui/gfx/render_text_unittest.cc (left):

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest....
ui/gfx/render_text_unittest.cc:425: { L"012" L"abc"                  ,
L"012a\x2026"               },
Why does that matter? 012 is just some random string here.

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest....
ui/gfx/render_text_unittest.cc:431: { L"0123\x0915\x093f" L"abcd"          ,
L"0123\x0915\x2026"          },
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.

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest....
ui/gfx/render_text_unittest.cc:436: { L"012\xF0\x9D\x84\x9E"               ,
L"012\xF0\x2026"             },
I think I will be cautious :)

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

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest.cc
File ui/gfx/render_text_unittest.cc (left):

https://codereview.chromium.org/103493003/diff/1/ui/gfx/render_text_unittest....
ui/gfx/render_text_unittest.cc:425: { L"012" L"abc"                  ,
L"012a\x2026"               },
On 2013/12/13 21:05:59, Anuj wrote:
> Why does that matter? 012 is just some random string here.

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.