Commit 203f0f8c authored by Emily Stark's avatar Emily Stark Committed by Commit Bot

Simplified domain: fix registrable domain substring finding

Previously, we were searching for the registrable domain from the left
end of the text and using that as our simplified domain bounds. This
is incorrect in the case that the registrable domain is also a
subdomain, e.g. example.com.example.com. Now we instead search from
the right end of the hostname.

Bug: 1111902f
Change-Id: I0fe97036986688b19f3a2315a32f413c8bf50b68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343572
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796191}
parent b90c6a2f
...@@ -2417,7 +2417,7 @@ gfx::Range OmniboxViewViews::GetSimplifiedDomainBounds( ...@@ -2417,7 +2417,7 @@ gfx::Range OmniboxViewViews::GetSimplifiedDomainBounds(
} }
size_t simplified_domain_pos = size_t simplified_domain_pos =
text.find(base::ASCIIToUTF16(simplified_domain)); text.rfind(base::ASCIIToUTF16(simplified_domain), host.end());
DCHECK_NE(simplified_domain_pos, std::string::npos); DCHECK_NE(simplified_domain_pos, std::string::npos);
ranges_surrounding_simplified_domain->emplace_back(0, simplified_domain_pos); ranges_surrounding_simplified_domain->emplace_back(0, simplified_domain_pos);
ranges_surrounding_simplified_domain->emplace_back(host.end(), text.size()); ranges_surrounding_simplified_domain->emplace_back(host.end(), text.size());
......
...@@ -225,6 +225,8 @@ class OmniboxViewViews : public OmniboxView, ...@@ -225,6 +225,8 @@ class OmniboxViewViews : public OmniboxView,
FRIEND_TEST_ALL_PREFIXES( FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest, OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest,
UnsetAlwaysShowFullURLs); UnsetAlwaysShowFullURLs);
FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsRevealOnHoverTest,
RegistrableDomainRepeated);
FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest, FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest,
EmitAccessibilityEvents); EmitAccessibilityEvents);
// TODO(tommycli): Remove the rest of these friends after porting these // TODO(tommycli): Remove the rest of these friends after porting these
......
...@@ -2975,3 +2975,40 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, AfterBlur) { ...@@ -2975,3 +2975,40 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, AfterBlur) {
ASSERT_TRUE(elide_animation); ASSERT_TRUE(elide_animation);
EXPECT_TRUE(elide_animation->IsAnimating()); EXPECT_TRUE(elide_animation->IsAnimating());
} }
// Tests that registrable domain elision properly handles the case when the
// registrable domain appears as a subdomain, e.g. test.com.test.com.
TEST_P(OmniboxViewViewsRevealOnHoverTest, RegistrableDomainRepeated) {
// This test only applies when the URL is elided to the registrable domain.
if (!ShouldElideToRegistrableDomain())
return;
const base::string16 kRepeatedRegistrableDomainUrl =
base::ASCIIToUTF16("https://example.com.example.com/foo");
gfx::Range registrable_domain_and_path_range(
20 /* "https://www.example.com." */,
kRepeatedRegistrableDomainUrl.size());
location_bar_model()->set_url(GURL(kRepeatedRegistrableDomainUrl));
location_bar_model()->set_url_for_display(kRepeatedRegistrableDomainUrl);
omnibox_view()->model()->ResetDisplayTexts();
omnibox_view()->RevertAll();
// Call OnThemeChanged() to create the animations.
omnibox_view()->OnThemeChanged();
ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
omnibox_view(), base::ASCIIToUTF16("https://"),
base::ASCIIToUTF16("example.com."),
base::ASCIIToUTF16("https://example.com.example.com"),
base::ASCIIToUTF16("/foo"), ShouldElideToRegistrableDomain()));
// Check that the domain is elided up to the second instance of "example.com",
// not the first.
gfx::Rect registrable_domain_and_path;
for (const auto& rect : omnibox_view()->GetRenderText()->GetSubstringBounds(
registrable_domain_and_path_range)) {
registrable_domain_and_path.Union(rect);
}
EXPECT_EQ(omnibox_view()->GetRenderText()->display_rect().x(),
registrable_domain_and_path.x());
}
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