Commit ed2c4514 authored by Tommy C. Li's avatar Tommy C. Li Committed by Commit Bot

Omnibox UI: Don't elide "m" subdomains on Desktop.

Eliding the "m" subdomain on Desktop can be confusing, since users
would generally want to know if they are unintentionally on the mobile
site on Desktop.

After this CL, we only elide the "m" subdomain on Android and iOS.

Bug: 875669
Change-Id: Ifededfce720444e5c9e85ffc97c92c8cee530937
Reviewed-on: https://chromium-review.googlesource.com/1185904Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586416}
parent de2ae649
......@@ -10,6 +10,7 @@
#include "base/macros.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
......@@ -69,11 +70,20 @@ struct TestItem {
"www.google.com/search?q=tractor+supply",
"google.com/search?q=tractor+supply",
},
#if defined(OS_ANDROID)
{
GURL("https://m.google.ca/search?q=tractor+supply"),
"https://m.google.ca/search?q=tractor+supply",
"google.ca/search?q=tractor+supply",
},
#else // !defined(OS_ANDROID)
// 'm' is not elided on Desktop.
{
GURL("https://m.google.ca/search?q=tractor+supply"),
"https://m.google.ca/search?q=tractor+supply",
"m.google.ca/search?q=tractor+supply",
},
#endif
};
} // namespace
......
......@@ -3906,10 +3906,11 @@ TEST_F(HistoryBackendTest, RedirectScoring) {
// The HTTPS URL should accrue the typed count, even if it removes a trivial
// subdomain.
const char* redirect3[] = {"http://m.foo3.com", "https://foo3.com", nullptr};
const char* redirect3[] = {"http://www.foo3.com", "https://foo3.com",
nullptr};
AddRedirectChainWithTransitionAndTime(redirect3, 3, ui::PAGE_TRANSITION_TYPED,
base::Time::Now());
ASSERT_TRUE(backend_->GetURL(GURL("http://m.foo3.com"), &url_row));
ASSERT_TRUE(backend_->GetURL(GURL("http://www.foo3.com"), &url_row));
EXPECT_EQ(0, url_row.typed_count());
ASSERT_TRUE(backend_->GetURL(GURL("https://foo3.com"), &url_row));
EXPECT_EQ(1, url_row.typed_count());
......@@ -4241,7 +4242,7 @@ TEST(FormatUrlForRedirectComparisonTest, TestUrlFormatting) {
FormatUrlForRedirectComparison(url2));
// Tests that the formatter removes repeated trivial subdomains.
GURL url3("http://m.www.www.baz.com/");
GURL url3("http://www.www.baz.com/");
EXPECT_EQ(base::ASCIIToUTF16("baz.com/"),
FormatUrlForRedirectComparison(url3));
}
......
......@@ -281,8 +281,8 @@ TEST(AutocompleteMatchTest, FormatUrlForSuggestionDisplay) {
{"https://google.com", true, false, false, L"https://google.com"},
// Test the |preserve_subdomain| parameter.
{"http://www.m.google.com", false, false, false, L"google.com"},
{"http://www.m.google.com", false, true, false, L"www.m.google.com"},
{"http://www.google.com", false, false, false, L"google.com"},
{"http://www.google.com", false, true, false, L"www.google.com"},
// Test that paths are preserved in the default case.
{"http://google.com/foobar", false, false, false, L"google.com/foobar"},
......
......@@ -17,6 +17,7 @@
#include "base/strings/utf_offset_string_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_local_storage.h"
#include "build/build_config.h"
#include "components/url_formatter/idn_spoof_checker.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "third_party/icu/source/common/unicode/uidna.h"
......@@ -67,6 +68,20 @@ class HostComponentTransform : public AppendComponentTransform {
: trim_trivial_subdomains_(trim_trivial_subdomains) {}
private:
static bool IsTrivialSubdomain(base::StringPiece subdomain) {
if (subdomain == "www")
return true;
#if defined(OS_ANDROID) || defined(OS_IOS)
// Eliding the "m" subdomain on Desktop can be confusing, since users would
// generally want to know if they are unintentionally on the mobile site.
if (subdomain == "m")
return true;
#endif
return false;
}
base::string16 Execute(
const std::string& component_text,
base::OffsetAdjuster::Adjustments* adjustments) const override {
......@@ -96,7 +111,7 @@ class HostComponentTransform : public AppendComponentTransform {
while (tokenizer.GetNext()) {
// Append delimiters and non-trivial subdomains to the new subdomain part.
if (tokenizer.token_is_delim() ||
(tokenizer.token() != "m" && tokenizer.token() != "www")) {
!IsTrivialSubdomain(tokenizer.token_piece())) {
transformed_subdomain += tokenizer.token();
continue;
}
......
......@@ -14,6 +14,7 @@
#include "base/strings/string_piece.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "components/url_formatter/idn_spoof_checker.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
......@@ -1018,6 +1019,7 @@ TEST(UrlFormatterTest, IDNToUnicode) {
TEST(UrlFormatterTest, FormatUrl) {
FormatUrlTypes default_format_type = kFormatUrlOmitUsernamePassword;
// clang-format off
const UrlTestData tests[] = {
{"Empty URL", "", default_format_type, net::UnescapeRule::NORMAL, L"", 0},
......@@ -1220,29 +1222,32 @@ TEST(UrlFormatterTest, FormatUrl) {
net::UnescapeRule::NORMAL, L"https://ftp.google.com/", 8},
// -------- omit trivial subdomains --------
{"omit trivial subdomains - trim www", "http://www.google.com/",
kFormatUrlOmitTrivialSubdomains, net::UnescapeRule::NORMAL,
L"http://google.com/", 7},
#if defined(OS_ANDROID) || defined(OS_IOS)
{"omit trivial subdomains - trim m", "http://m.google.com/",
kFormatUrlOmitTrivialSubdomains, net::UnescapeRule::NORMAL,
L"http://google.com/", 7},
kFormatUrlOmitTrivialSubdomains, net::UnescapeRule::NORMAL,
L"http://google.com/", 7},
{"omit trivial subdomains - trim m and www", "http://m.www.google.com/",
kFormatUrlOmitTrivialSubdomains, net::UnescapeRule::NORMAL,
L"http://google.com/", 7},
kFormatUrlOmitTrivialSubdomains, net::UnescapeRule::NORMAL,
L"http://google.com/", 7},
{"omit trivial subdomains - trim m from middle",
"http://en.m.wikipedia.org/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://en.wikipedia.org/", 7},
{"omit trivial subdomains - exclude private registries",
"http://www.blogspot.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://blogspot.com/", 7},
{"omit trivial subdomains - don't do blind substring matches for www",
"http://wwww.google.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://wwww.google.com/", 7},
"http://en.m.wikipedia.org/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://en.wikipedia.org/", 7},
#else // !(defined(OS_ANDROID) || defined(OS_IOS))
{"omit trivial subdomains - don't trim m on desktop",
"http://m.google.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://m.google.com/", 7},
{"omit trivial subdomains - trim www but leave m on desktop",
"http://m.www.google.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://m.google.com/", 7},
{"omit trivial subdomains - don't trim m from middle on desktop",
"http://en.m.wikipedia.org/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://en.m.wikipedia.org/", 7},
#endif
{"omit trivial subdomains - don't do blind substring matches for m",
"http://foom.google.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://foom.google.com/", 7},
{"omit trivial subdomains - don't crash on multiple delimiters",
"http://www...m..foobar...google.com/", kFormatUrlOmitTrivialSubdomains,
"http://www....foobar...google.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://...foobar...google.com/", 7},
{"omit trivial subdomains - sanity check for ordinary subdomains",
......@@ -1255,7 +1260,7 @@ TEST(UrlFormatterTest, FormatUrl) {
"http://google.com/www.m.foobar", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, L"http://google.com/www.m.foobar", 7},
{"omit trivial subdomains - sanity check for IDN",
"http://www.xn--cy2a840a.m.xn--cy2a840a.com",
"http://www.xn--cy2a840a.www.xn--cy2a840a.com",
kFormatUrlOmitTrivialSubdomains, net::UnescapeRule::NORMAL,
L"http://\x89c6\x9891.\x89c6\x9891.com/", 7},
......@@ -1316,6 +1321,7 @@ TEST(UrlFormatterTest, FormatUrl) {
kFormatUrlOmitDefaults | kFormatUrlTrimAfterHost,
net::UnescapeRule::NORMAL, L"google.com", 0},
};
// clang-format on
for (size_t i = 0; i < arraysize(tests); ++i) {
size_t prefix_len;
......@@ -1728,18 +1734,27 @@ TEST(UrlFormatterTest, FormatUrlWithOffsets) {
net::UnescapeRule::NORMAL, omit_https_with_auth_offsets);
const size_t strip_trivial_subdomains_offsets_1[] = {
0, 1, 2, 3, 4, 5, 6, 7, kNpos, kNpos, kNpos, 7, kNpos, 7,
8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21};
0, 1, 2, 3, 4, 5, 6, 7, kNpos, kNpos, kNpos, 7, 8,
9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21};
CheckAdjustedOffsets(
"http://www.m.google.com/foo/", kFormatUrlOmitTrivialSubdomains,
"http://www.google.com/foo/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, strip_trivial_subdomains_offsets_1);
#if defined(OS_ANDROID) || defined(OS_IOS)
const size_t strip_trivial_subdomains_offsets_2[] = {
0, 1, 2, 3, 4, 5, 6, 7, kNpos, 7, 8, 9,
10, kNpos, kNpos, kNpos, 10, 11, 12, 13, 14, 15, 16, 17};
CheckAdjustedOffsets(
"http://m.en.www.foo.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, strip_trivial_subdomains_offsets_2);
#else // !(defined(OS_ANDROID) || defined(OS_IOS))
const size_t strip_trivial_subdomains_offsets_2[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,
12, kNpos, kNpos, kNpos, 12, 13, 14, 15, 16, 17, 18, 19};
CheckAdjustedOffsets(
"http://m.en.www.foo.com/", kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::NORMAL, strip_trivial_subdomains_offsets_2);
#endif
const size_t strip_trivial_subdomains_from_idn_offsets[] = {
0, 1, 2, 3, 4, 5, 6, 7, kNpos, kNpos,
......
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