chrome/browser/ui/elide_url_unittest.cc - Issue 1147363005: Create FormatUrlForSecurityDisplay helper function.

Unified Diff: chrome/browser/ui/elide_url_unittest.cc

Issue 1147363005: Create FormatUrlForSecurityDisplay helper function. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Rename to FormatUrlForSecurityDisplay. Created 5 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: chrome/browser/ui/elide_url_unittest.cc

diff --git a/chrome/browser/ui/elide_url_unittest.cc b/chrome/browser/ui/elide_url_unittest.cc

index a2cec9e82b490b05b77a266ea77b8a3d874592a7..40969b7119fbd39d7a8625f46748627baa65a1d6 100644

--- a/chrome/browser/ui/elide_url_unittest.cc

+++ b/chrome/browser/ui/elide_url_unittest.cc

@@ -204,4 +204,284 @@ TEST(TextEliderTest, TestHostEliding) {

ElideHost(GURL("http://foo.bar"), gfx::FontList(), 2));

}

+struct OriginTestData {

+ const char* const description;

+ const char* const input;

+ const char* const languages;

msw 2015/06/02 23:45:16 afaict, the languages never affect the output for

palmer 2015/06/03 01:01:46 I'm not sure I know how to do that. :) Ultimately,

msw 2015/06/03 01:19:21 Maybe ask Brett about that. You can follow my firs

Maybe ask Brett about that. You can follow my first recommendation: A) Omit |languages| from this struct and the test cases. B) Delete all the tests cases "with languages" (they're dups). C) Make the loop check with and without languages like: base::string16 formatted = FormatUrlForSecurityDisplay(GURL(tests[i].input), std::string()); EXPECT_EQ(base::WideToUTF16(tests[i].output), formatted) << tests[i].description; base::string16 formatted_with_languages = FormatUrlForSecurityDisplay(GURL(tests[i].input), "en-US,zh-TW,zh"); // Or choose some other set of languages here... EXPECT_EQ(base::WideToUTF16(tests[i].output), formatted_with_languages) << tests[i].description;

palmer 2015/06/03 21:24:38 Done.

On 2015/06/03 01:19:21, msw wrote: > On 2015/06/03 01:01:46, palmer wrote: > > On 2015/06/02 23:45:16, msw wrote: > > > afaict, the languages never affect the output for any of these test cases > (all > > > outputs match the corresponding cases with no languages specified). Can you > > make > > > the loop test an empty languages output against a constant (or per-case...) > > > non-empty languages output to avoid all the duplicate cases? Can you add one > > or > > > more test cases (in a separate test?) that verifies some output is actually > > > affected by the languages string? (net::FormatURL says it'll support unicode > > IDN > > > host names only with certain languages). > > > > I'm not sure I know how to do that. :) Ultimately, we end up at this code (in > > net/base/net_util_icu.cc): > > > > 360 // TODO(brettw) bug 734373: check the scripts for each host component and > > 361 // don't un-IDN-ize if there is more than one. Alternatively, only IDN for > > 362 // scripts that the user has installed. For now, just put the entire > > 363 // path through IDN. Maybe this feature can be implemented in ICU itself? > > 364 // > > 365 // We may want to skip this step in the case of file URLs to allow unicode > > 366 // UNC hostnames regardless of encodings. > > 367 base::string16 IDNToUnicodeWithAdjustments( > > 368 const std::string& host, > > 369 const std::string& languages, > > 370 base::OffsetAdjuster::Adjustments* adjustments) { > > > > which references an ancient bug (in the Google internal bug tracker). It seems > > to describe the problem that there is not yet a clear policy for how we > support > > Unicode code host names? > > Maybe ask Brett about that. You can follow my first recommendation: > A) Omit |languages| from this struct and the test cases. > B) Delete all the tests cases "with languages" (they're dups). > C) Make the loop check with and without languages like: > base::string16 formatted = FormatUrlForSecurityDisplay(GURL(tests[i].input), > std::string()); > EXPECT_EQ(base::WideToUTF16(tests[i].output), formatted) << > tests[i].description; > base::string16 formatted_with_languages = > FormatUrlForSecurityDisplay(GURL(tests[i].input), "en-US,zh-TW,zh"); // Or > choose some other set of languages here... > EXPECT_EQ(base::WideToUTF16(tests[i].output), formatted_with_languages) << > tests[i].description;

Done.

+ const wchar_t* const output;

+};

+TEST(TextEliderTest, FormatUrlForSecurityDisplay) {

+ const OriginTestData tests[] = {

+ {"Empty URL", "", "", L""},

+ {"Empty URL with languages", "", "en-US,zh-TW,zh", L""},

msw 2015/06/02 23:45:16 nit: every other desc including "with languages" h

palmer 2015/06/03 01:01:47 Done.

+ {"HTTP URL", "http://www.google.com/", "", L"http://www.google.com"},

+ {"HTTP URL, with languages",

+ "http://www.google.com/",

+ "en-US,en,am",

+ L"http://www.google.com"},

+ {"HTTPS URL", "https://www.google.com/", "", L"https://www.google.com"},

+ {"HTTPS URL, with languages",

+ "https://www.google.com/",

+ "en-US,en,am,zh",

+ L"https://www.google.com"},

+ {"Standard HTTP port",

+ "http://www.google.com:80/",

+ "",

+ L"http://www.google.com"},

+ {"Standard HTTP port, with languages",

+ "http://www.google.com:80/",

+ "en-US,en,am,zh",

+ L"http://www.google.com"},

+ {"Standard HTTPS port",

+ "https://www.google.com:443/",

+ "",

+ L"https://www.google.com"},

+ {"Standard HTTPS port, with languages",

+ "https://www.google.com:443/",

+ "en-US,en,am,zh-TW",

+ L"https://www.google.com"},

+ {"Standard HTTP port, IDN Chinese",

+ "http://\xe4\xb8\xad\xe5\x9b\xbd.icom.museum:80",

+ "",

+ L"http://xn--fiqs8s.icom.museum"},

+ {"Standard HTTP port, IDN Chinese, with languages",

+ "http://\xe4\xb8\xad\xe5\x9b\xbd.icom.museum:80",

+ "en-US,en,am,zh-TW",

+ L"http://xn--fiqs8s.icom.museum"},

+ {"HTTP URL, IDN Hebrew (RTL)",

+ "http://"

+ "\xd7\x90\xd7\x99\xd7\xa7\xd7\x95\xd7\xb4\xd7\x9d."

+ "\xd7\x99\xd7\xa9\xd7\xa8\xd7\x90\xd7\x9c.museum/",

+ "",

+ L"http://xn--4dbklr2c8d.xn--4dbrk0ce.museum"},

+ {"HTTP URL, IDN Hebrew (RTL), with languages",

+ "http://"

+ "\xd7\x90\xd7\x99\xd7\xa7\xd7\x95\xd7\xb4\xd7\x9d."

+ "\xd7\x99\xd7\xa9\xd7\xa8\xd7\x90\xd7\x9c.museum/",

+ "en-US,en,am,zh-TW",

+ L"http://xn--4dbklr2c8d.xn--4dbrk0ce.museum"},

+ {"HTTP URL with query string, IDN Arabic (RTL)",

+ "http://\xd9\x85\xd8\xb5\xd8\xb1.icom.museum/foo.html?yes=no",

+ "",

+ L"http://xn--wgbh1c.icom.museum"},

+ {"HTTP URL with query string, IDN Arabic (RTL), with languages",

+ "http://\xd9\x85\xd8\xb5\xd8\xb1.icom.museum/foo.html?yes=no",

+ "ar-EG,en,am,zh-TW",

+ L"http://xn--wgbh1c.icom.museum"},

+ {"Non-standard HTTP port",

+ "http://www.google.com:9000/",

+ "",

+ L"http://www.google.com:9000"},

+ {"Non-standard HTTP port, with languages",

+ "http://www.google.com:9000/",

+ "en-US,en,am,zh",

+ L"http://www.google.com:9000"},

+ {"Non-standard HTTPS port",

+ "https://www.google.com:9000/",

+ "",

+ L"https://www.google.com:9000"},

+ {"Non-standard HTTPS port, with languages",

+ "https://www.google.com:9000/",

+ "en,am",

+ L"https://www.google.com:9000"},

+ {"File URI",

+ "file:///usr/example/file.html",

+ "",

+ L"file:///usr/example/file.html"},

+ {"File URI, with languages",

+ "file:///usr/example/file.html",

+ "en-US,en,zh",

+ L"file:///usr/example/file.html"},

+ {"File URI with hostname",

+ "file://localhost/usr/example/file.html",

+ "",

+ L"file:///usr/example/file.html"},

+ {"File URI with hostname, with languages",

+ "file://localhost/usr/example/file.html",

+ "en-US,zh",

+ L"file:///usr/example/file.html"},

+ {"UNC File URI 1",

+ "file:///CONTOSO/accounting/money.xls",

+ "",

+ L"file:///CONTOSO/accounting/money.xls"},

+ {"UNC File URI 1, with languages",

+ "file:///CONTOSO/accounting/money.xls",

+ "zh-TW,en",

+ L"file:///CONTOSO/accounting/money.xls"},

+ {"UNC File URI 2",

+ "file:///C:/Program%20Files/Music/Web%20Sys/main.html?REQUEST=RADIO",

+ "",

+ L"file:///C:/Program%20Files/Music/Web%20Sys/main.html"},

+ {"UNC File URI 2, with languages",

+ "file:///C:/Program%20Files/Music/Web%20Sys/main.html?REQUEST=RADIO",

+ "ar-EG",

+ L"file:///C:/Program%20Files/Music/Web%20Sys/main.html"},

+ {"HTTP URL with path",

+ "http://www.google.com/test.html",

+ "",

+ L"http://www.google.com"},

+ {"HTTP URL with path, with languages",

+ "http://www.google.com/test.html",

+ "en-US,en,am,zh",

+ L"http://www.google.com"},

+ {"HTTPS URL with path",

+ "https://www.google.com/test.html",

+ "",

+ L"https://www.google.com"},

+ {"HTTPS URL with path, with languages",

+ "https://www.google.com/test.html",

+ "en-US,en,am,zh",

+ L"https://www.google.com"},

+ {"Unusual secure scheme (wss)",

+ "wss://www.google.com/",

+ "",

+ L"wss://www.google.com"},

+ {"Unusual secure scheme (wss), with languages",

+ "wss://www.google.com/",

+ "en-US,en,am,zh",

+ L"wss://www.google.com"},

+ {"Unusual non-secure scheme (gopher)",

+ "gopher://www.google.com/",

+ "",

+ L"gopher://www.google.com"},

+ {"Unusual non-secure scheme (gopher), with languages",

+ "gopher://www.google.com/",

+ "en-US",

+ L"gopher://www.google.com"},

+ {"Unlisted scheme (chrome)", "chrome://version", "", L"chrome://version"},

+ {"Unlisted scheme (chrome), with languages",

+ "chrome://version",

+ "en-US,en,am,zh",

+ L"chrome://version"},

+ {"HTTP IP address",

+ "http://173.194.65.103",

+ "",

+ L"http://173.194.65.103"},

+ {"HTTP IP address, with languages",

+ "http://173.194.65.103",

+ "en-US,en,am,zh",

+ L"http://173.194.65.103"},

+ {"HTTPS IP address",

+ "https://173.194.65.103",

+ "",

+ L"https://173.194.65.103"},

+ {"HTTPS IP address, with languages",

+ "https://173.194.65.103",

+ "en-US,en,am",

+ L"https://173.194.65.103"},

+ {"HTTP IPv6 address",

+ "http://[FE80:0000:0000:0000:0202:B3FF:FE1E:8329]/",

+ "",

+ L"http://[fe80::202:b3ff:fe1e:8329]"},

+ {"HTTP IPv6 address, with languages",

+ "http://[FE80:0000:0000:0000:0202:B3FF:FE1E:8329]/",

+ "zh",

+ L"http://[fe80::202:b3ff:fe1e:8329]"},

+ {"HTTPS IPv6 address with port",

+ "https://[2001:db8:0:1]:443/",

+ "",

+ L"https://[2001:db8:0:1]"},

+ {"HTTPS IPv6 address with port, with languages",

+ "https://[2001:db8:0:1]:443/",

+ "am,zh",

+ L"https://[2001:db8:0:1]"},

+ {"HTTPS IP address, non-default port, with languages",

msw 2015/06/02 23:45:16 nit: no corresponding case without languages

palmer 2015/06/03 01:01:46 Done.

+ "https://173.194.65.103:8443",

+ "en-US,en,am,zh",

+ L"https://173.194.65.103:8443"},

+ {"HTTP filesystem: URL with path",

+ "filesystem:http://www.google.com/temporary/test.html",

+ "",

+ L"filesystem:http://www.google.com"},

+ {"HTTP filesystem: URL with path, with languages",

+ "filesystem:http://www.google.com/temporary/test.html",

+ "en-US,en",

+ L"filesystem:http://www.google.com"},

+ {"File filesystem: URL with path",

+ "filesystem:file://localhost/temporary/stuff/test.html?z=fun&goat=billy",

+ "",

+ L"filesystem:file:///temporary/stuff/test.html"},

+ {"File filesystem: URL with path, with languages",

+ "filesystem:file://localhost/temporary/stuff/test.html?z=fun&goat=billy",

+ "en-US,en,am,zh",

+ L"filesystem:file:///temporary/stuff/test.html"},

+ {"Invalid scheme 1",

+ "twelve://www.cyber.org/wow.php",

+ "",

+ L"twelve://www.cyber.org/wow.php"},

+ {"Invalid scheme 1, with languages",

+ "twelve://www.cyber.org/wow.php",

+ "am,zh",

+ L"twelve://www.cyber.org/wow.php"},

+ {"Invalid scheme 2",

+ "://www.cyber.org/wow.php",

+ "",

+ L"://www.cyber.org/wow.php"},

+ {"Invalid scheme 2, with languages",

+ "://www.cyber.org/wow.php",

+ "en-US,zh",

+ L"://www.cyber.org/wow.php"},

+ {"Invalid host 1",

+ "https://www.cyber../wow.php",

+ "",

+ L"https://www.cyber.."},

+ {"Invalid host 1, with languages",

+ "https://www.cyber../wow.php",

+ "en-US,en,am",

+ L"https://www.cyber.."},

+ {"Invalid host 2",

+ "https://www...cyber/wow.php",

+ "",

+ L"https://www...cyber"},

+ {"Invalid host 2, with languages",

+ "https://www...cyber/wow.php",

+ "en-US,en,am,zh",

+ L"https://www...cyber"},

+ {"Invalid port 1",

+ "https://173.194.65.103:000",

+ "",

+ L"https://173.194.65.103:0"},

+ {"Invalid port 1, with languages",

+ "https://173.194.65.103:000",

+ "en-US,en",

+ L"https://173.194.65.103:0"},

+ {"Invalid port 2",

+ "https://173.194.65.103:gruffle",

+ "",

+ L"https://173.194.65.103:gruffle"},

+ {"Invalid port 2, with languages",

+ "https://173.194.65.103:gruffle",

+ "fr-FR",

+ L"https://173.194.65.103:gruffle"},

+ {"Invalid port 3",

+ "https://173.194.65.103:/hello.aspx",

+ "",

+ L"https://173.194.65.103"},

+ {"Invalid port 3, with languages",

+ "https://173.194.65.103:/hello.aspx",

+ "zh",

+ L"https://173.194.65.103"},

+ {"Trailing dot in DNS name",

msw 2015/06/02 23:45:16 nit: no corresponding case without languages, lang

palmer 2015/06/03 01:01:46 Done.

+ "https://www.example.com./get/goat",

+ "zh",

+ L"https://www.example.com."},

+ {"Blob URL",

msw 2015/06/02 23:45:16 nit: no corresponding case without languages, lang

palmer 2015/06/03 01:01:46 Done.

+ "blob:http%3A//www.html5rocks.com/4d4ff040-6d61-4446-86d3-13ca07ec9ab9",

+ "en-US,zh",

+ L"blob:http%3A//www.html5rocks.com/"

+ L"4d4ff040-6d61-4446-86d3-13ca07ec9ab9"},

+ };

+ for (size_t i = 0; i < arraysize(tests); ++i) {

+ base::string16 formatted =

+ FormatUrlForSecurityDisplay(GURL(tests[i].input), tests[i].languages);

+ EXPECT_EQ(base::WideToUTF16(tests[i].output), formatted)

+ << tests[i].description;

+ }

+ base::string16 formatted = FormatUrlForSecurityDisplay(GURL(), std::string());

+ EXPECT_EQ(base::string16(), formatted)

+ << "Explicitly test the 0-argument GURL constructor";

} // namespace

« chrome/browser/ui/elide_url.cc ('K') | « chrome/browser/ui/elide_url.cc ('k') | no next file » | no next file with comments »