Commit 8f462482 authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Rewrite ElideText unittests

This CL is rewriting the ElideText unittests.
The goal of this rewrite is to allow adding more unittests.
  * ELIDE_HEAD
  * ELIDE_EMAIL
  * ELIDE_MIDDLE
  * Eliding with styles

It is now using a templated tests to ease debugging.

A change to the way glyph_width_for_test_ work was needed.
Zero width codepoints will stay zero width, even if a
glyph width is provided. This is required to ensures
RLM and LRM (directional marker) stay with width zero.
Otherwise a DCHECK is failing since the text can be larger
with eliding (2 glyphs instead of 1 glyph).

Bug: 1025561
Change-Id: I19f18fb99f52edbbb59d170f7b47bef3a3f3de5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1975056
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726406}
parent 360eb8f5
...@@ -1337,10 +1337,13 @@ void ShapeRunWithFont(const ShapeRunWithFontInput& in, ...@@ -1337,10 +1337,13 @@ void ShapeRunWithFont(const ShapeRunWithFontInput& in,
const SkScalar y_offset = const SkScalar y_offset =
HarfBuzzUnitsToSkiaScalar(hb_positions[i].y_offset); HarfBuzzUnitsToSkiaScalar(hb_positions[i].y_offset);
out->positions[i].set(out->width + x_offset, -y_offset); out->positions[i].set(out->width + x_offset, -y_offset);
out->width += (in.glyph_width_for_test > 0)
? in.glyph_width_for_test if (in.glyph_width_for_test == 0)
: HarfBuzzUnitsToFloat(hb_positions[i].x_advance) + out->width +=
in.glyph_spacing; HarfBuzzUnitsToFloat(hb_positions[i].x_advance) + in.glyph_spacing;
else if (hb_positions[i].x_advance) // Leave zero-width glyphs alone.
out->width += in.glyph_width_for_test;
// Round run widths if subpixel positioning is off to match native behavior. // Round run widths if subpixel positioning is off to match native behavior.
if (!in.render_params.subpixel_positioning) if (!in.render_params.subpixel_positioning)
out->width = std::round(out->width); out->width = std::round(out->width);
......
...@@ -1751,92 +1751,109 @@ INSTANTIATE_TEST_SUITE_P(ItemizeTextToRunsEmoji, ...@@ -1751,92 +1751,109 @@ INSTANTIATE_TEST_SUITE_P(ItemizeTextToRunsEmoji,
::testing::ValuesIn(kEmojiRunListCases), ::testing::ValuesIn(kEmojiRunListCases),
RenderTextTestWithRunListCase::ParamInfoToString); RenderTextTestWithRunListCase::ParamInfoToString);
TEST_F(RenderTextTest, ElidedText) { struct ElideTextCase {
// TODO(skanuj) : Add more test cases for following const char* test_name;
// - RenderText styles.
// - Cross interaction of truncate, elide and obscure.
// - ElideText tests from text_elider.cc.
struct {
const wchar_t* text; const wchar_t* text;
const wchar_t* display_text; const wchar_t* display_text;
const bool elision_expected; };
} cases[] = {
// Strings shorter than the elision width should be laid out in full. class RenderTextTestWithElideTextCase
{L"", L"", false}, : public RenderTextTest,
{L"M", L"", false}, public ::testing::WithParamInterface<ElideTextCase> {
{L" . ", L" . ", false}, // a wide kWeak public:
{L"abc", L"abc", false}, // a wide kLtr static std::string ParamInfoToString(
{L"\u05d0\u05d1\u05d2", L"\u05d0\u05d1\u05d2", false}, // a wide kRtl ::testing::TestParamInfo<ElideTextCase> param_info) {
{L"a\u05d0\u05d1", L"a\u05d0\u05d1", false}, // a wide kLtrRtl return param_info.param.test_name;
{L"a\u05d1b", L"a\u05d1b", false}, // a wide kLtrRtlLtr }
{L"\u05d0\u05d1a", L"\u05d0\u05d1a", false}, // a wide kRtlLtr };
{L"\u05d0a\u05d1", L"\u05d0a\u05d1", false}, // a wide kRtlLtrRtl
// Strings as long as the elision width should be laid out in full. TEST_P(RenderTextTestWithElideTextCase, ElideText) {
{L"012ab", L"012ab", false}, // This test requires glyphs to be the same width.
// Long strings should be elided with an ellipsis appended at the end. constexpr int kGlyphWidth = 10;
{L"012abc", L"012a\u2026", true}, SetGlyphWidth(kGlyphWidth);
{L"012ab\u05d0\u05d1", L"012a\u2026", true},
{L"012a\u05d1b", L"012a\u2026", true}, ElideTextCase param = GetParam();
const base::string16 text = WideToUTF16(param.text);
const base::string16 display_text = WideToUTF16(param.display_text);
// Retrieve the display_text width without eliding.
RenderTextHarfBuzz* render_text = GetRenderText();
render_text->SetText(display_text);
const int expected_width = render_text->GetContentWidth();
// Set the text and the eliding behavior.
render_text->SetText(text);
render_text->SetDisplayRect(
Rect(0, 0, expected_width + kGlyphWidth / 2, 100));
render_text->SetElideBehavior(ELIDE_TAIL);
render_text->SetWhitespaceElision(false);
const int elided_width = render_text->GetContentWidth();
EXPECT_EQ(text, render_text->text());
EXPECT_EQ(display_text, render_text->GetDisplayText());
EXPECT_EQ(elided_width, expected_width);
}
const ElideTextCase kElideTailTextCases[] = {
{"empty", L"", L""},
{"letter_m_tail0", L"M", L""},
{"letter_m_tail1", L"M", L"M"},
{"letter_weak_3", L" . ", L" . "},
{"letter_weak_2", L" . ", L" \u2026"},
{"no_eliding", L"012ab", L"012ab"},
{"ltr_3", L"abc", L"abc"},
{"ltr_2", L"abc", L"a\u2026"},
{"ltr_1", L"abc", L"\u2026"},
{"ltr_0", L"abc", L""},
{"rtl_3", L"\u05d0\u05d1\u05d2", L"\u05d0\u05d1\u05d2"},
{"rtl_2", L"\u05d0\u05d1\u05d2", L"\u05d0\u2026"},
{"rtl_1", L"\u05d0\u05d1\u05d2", L"\u2026\x200E"},
{"rtl_0", L"\u05d0\u05d1\u05d2", L""},
{"ltr_rtl_5", L"abc\u05d0\u05d1\u05d2", L"abc\u05d0\u2026\x200F"},
{"ltr_rtl_4", L"abc\u05d0\u05d1\u05d2", L"abc\u2026"},
{"ltr_rtl_3", L"abc\u05d0\u05d1\u05d2", L"ab\u2026"},
{"rtl_ltr_5", L"\u05d0\u05d1\u05d2abc", L"\u05d0\u05d1\u05d2a\u2026\x200E"},
{"rtl_ltr_4", L"\u05d0\u05d1\u05d2abc", L"\u05d0\u05d1\u05d2\u2026"},
{"rtl_ltr_3", L"\u05d0\u05d1\u05d2abc", L"\u05d0\u05d1\u2026"},
{"bidi_1", L"012a\u05d1b\u05d1c", L"012a\u2026"},
{"bidi_2", L"012a\u05d1b\u05d1c", L"012a\u05d1\u2026\x200F"},
{"bidi_3", L"012a\u05d1b\u05d1c", L"012a\u05d1b\u2026\x200F"},
// No RLM marker added as digits (012) have weak directionality. // No RLM marker added as digits (012) have weak directionality.
{L"01\u05d0\u05d1\u05d2", L"01\u05d0\u05d1\u2026", true}, {"no_rlm", L"01\u05d0\u05d1\u05d2", L"01\u05d0\u2026"},
// RLM marker added as "ab" have strong LTR directionality. // RLM marker added as "ab" have strong LTR directionality.
{L"ab\u05d0\u05d1\u05d2", L"ab\u05d0\u05d1\u2026\u200f", true}, {"with_rlm", L"ab\u05d0\u05d1\u05d2cd", L"ab\u05d0\u05d1\u2026\u200f"},
// Test surrogate pairs. The first pair 𝄞 'MUSICAL SYMBOL G CLEF' U+1D11E // Test surrogate pairs. The first pair 𝄞 'MUSICAL SYMBOL G CLEF' U+1D11E
// should be kept, and the second pair 𝄢 'MUSICAL SYMBOL F CLEF' U+1D122 // should be kept, and the second pair 𝄢 'MUSICAL SYMBOL F CLEF' U+1D122
// should be removed. No surrogate pair should be partially elided. // should be removed. No surrogate pair should be partially elided.
// Windows requires wide strings for \Unnnnnnnn universal character names. {"surrogate", L"0123\U0001D11E\U0001D122x", L"0123\U0001D11E\u2026"},
{L"0123\U0001D11E\U0001D122", L"0123\U0001D11E\u2026", true},
// Test combining character sequences. U+0915 U+093F forms a compound // Test combining character sequences. U+0915 U+093F forms a compound
// glyph, as does U+0915 U+0942. The first should be kept; the second // glyph, as does U+0915 U+0942. The first should be kept; the second
// removed. No combining sequence should be partially elided. // removed. No combining sequence should be partially elided.
{L"0123\u0915\u093f\u0915\u0942", L"0123\u0915\u093f\u2026", true}, {"combining", L"0123\u0915\u093f\u0915\u0942456",
L"0123\u0915\u093f\u2026"},
// U+05E9 U+05BC U+05C1 U+05B8 forms a four-character compound glyph. // U+05E9 U+05BC U+05C1 U+05B8 forms a four-character compound glyph.
// It should be either fully elided, or not elided at all. If completely // It should be either fully elided, or not elided at all. If completely
// elided, an LTR Mark (U+200E) should be added. // elided, an LTR Mark (U+200E) should be added.
{L"0\u05e9\u05bc\u05c1\u05b8", L"0\u05e9\u05bc\u05c1\u05b8", false}, {"grapheme1", L"0\u05e9\u05bc\u05c1\u05b8", L"0\u05e9\u05bc\u05c1\u05b8"},
{L"0\u05e9\u05bc\u05c1\u05b8", L"0\u2026\u200E", true}, {"grapheme2", L"0\u05e9\u05bc\u05c1\u05b8abc", L"0\u2026\u200E"},
{L"01\u05e9\u05bc\u05c1\u05b8", L"01\u2026\u200E", true}, {"grapheme3", L"01\u05e9\u05bc\u05c1\u05b8abc", L"01\u2026\u200E"},
{L"012\u05e9\u05bc\u05c1\u05b8", L"012\u2026\u200E", true}, {"grapheme4", L"012\u05e9\u05bc\u05c1\u05b8abc", L"012\u2026\u200E"},
// 𝄞 (U+1D11E, MUSICAL SYMBOL G CLEF) should be fully elided. // 𝄞 (U+1D11E, MUSICAL SYMBOL G CLEF) should be fully elided.
// Windows requires wide strings for \Unnnnnnnn universal character names. {"emoji", L"012\U0001D11Ex", L"012\u2026"},
{L"012\U0001D11E", L"012\u2026", true}, };
};
auto expected_render_text = std::make_unique<RenderTextHarfBuzz>();
expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
expected_render_text->SetDisplayRect(Rect(0, 0, 9999, 100));
RenderText* render_text = GetRenderText(); INSTANTIATE_TEST_SUITE_P(ElideTail,
render_text->SetFontList(FontList("serif, Sans serif, 12px")); RenderTextTestWithElideTextCase,
render_text->SetElideBehavior(ELIDE_TAIL); ::testing::ValuesIn(kElideTailTextCases),
RenderTextTestWithElideTextCase::ParamInfoToString);
for (size_t i = 0; i < base::size(cases); i++) {
SCOPED_TRACE(base::StringPrintf("Testing cases[%" PRIuS "] '%ls'", i,
cases[i].text));
// Compute expected width
expected_render_text->SetText(WideToUTF16(cases[i].display_text));
int expected_width = expected_render_text->GetContentWidth();
base::string16 input = WideToUTF16(cases[i].text);
// Extend the input text to ensure that it is wider than the display_text,
// and so it will get elided.
if (cases[i].elision_expected)
input.append(UTF8ToUTF16(" MMMMMMMMMMM"));
render_text->SetText(input);
render_text->SetDisplayRect(Rect(0, 0, expected_width, 100));
EXPECT_EQ(input, render_text->text());
EXPECT_EQ(WideToUTF16(cases[i].display_text),
render_text->GetDisplayText());
expected_render_text->SetText(base::string16());
}
}
TEST_F(RenderTextTest, ElidedText_NoTrimWhitespace) { TEST_F(RenderTextTest, ElidedText_NoTrimWhitespace) {
const int kGlyphWidth = 10; // This test requires glyphs to be the same width.
constexpr int kGlyphWidth = 10;
SetGlyphWidth(kGlyphWidth);
RenderText* render_text = GetRenderText(); RenderText* render_text = GetRenderText();
gfx::test::RenderTextTestApi render_text_test_api(render_text); gfx::test::RenderTextTestApi render_text_test_api(render_text);
render_text_test_api.SetGlyphWidth(kGlyphWidth);
render_text->SetElideBehavior(ELIDE_TAIL); render_text->SetElideBehavior(ELIDE_TAIL);
render_text->SetWhitespaceElision(false); render_text->SetWhitespaceElision(false);
...@@ -1867,7 +1884,6 @@ TEST_F(RenderTextTest, ElidedText_NoTrimWhitespace) { ...@@ -1867,7 +1884,6 @@ TEST_F(RenderTextTest, ElidedText_NoTrimWhitespace) {
TEST_F(RenderTextTest, ElidedObscuredText) { TEST_F(RenderTextTest, ElidedObscuredText) {
auto expected_render_text = std::make_unique<RenderTextHarfBuzz>(); auto expected_render_text = std::make_unique<RenderTextHarfBuzz>();
expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
expected_render_text->SetDisplayRect(Rect(0, 0, 9999, 100)); expected_render_text->SetDisplayRect(Rect(0, 0, 9999, 100));
const base::char16 elided_obscured_text[] = { const base::char16 elided_obscured_text[] = {
RenderText::kPasswordReplacementChar, RenderText::kPasswordReplacementChar,
...@@ -1875,7 +1891,6 @@ TEST_F(RenderTextTest, ElidedObscuredText) { ...@@ -1875,7 +1891,6 @@ TEST_F(RenderTextTest, ElidedObscuredText) {
expected_render_text->SetText(elided_obscured_text); expected_render_text->SetText(elided_obscured_text);
RenderText* render_text = GetRenderText(); RenderText* render_text = GetRenderText();
render_text->SetFontList(FontList("serif, Sans serif, 12px"));
render_text->SetElideBehavior(ELIDE_TAIL); render_text->SetElideBehavior(ELIDE_TAIL);
render_text->SetDisplayRect( render_text->SetDisplayRect(
Rect(0, 0, expected_render_text->GetContentWidth(), 100)); Rect(0, 0, expected_render_text->GetContentWidth(), 100));
...@@ -2151,7 +2166,7 @@ TEST_F(RenderTextTest, ElidedStyledTextRtl) { ...@@ -2151,7 +2166,7 @@ TEST_F(RenderTextTest, ElidedStyledTextRtl) {
break; break;
} }
} }
} // namespace gfx }
TEST_F(RenderTextTest, ElidedEmail) { TEST_F(RenderTextTest, ElidedEmail) {
RenderText* render_text = GetRenderText(); RenderText* render_text = GetRenderText();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment