Commit 3a1b73b8 authored by Emily Stark's avatar Emily Stark Committed by Commit Bot

Simplified domains: do not elide localhost URLs

Localhost URLs are highly unlikely to be phishing/social engineering,
and it's especially useful for developers using localhost to be able
to see port and path by default.

Bug: 1112816
Change-Id: I245cbb5cacffebdb04cbb7840629c0ab68702883
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2367802Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800325}
parent c8351891
......@@ -61,6 +61,7 @@
#include "extensions/common/constants.h"
#include "net/base/escape.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "net/base/url_util.h"
#include "third_party/metrics_proto/omnibox_event.pb.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/accessibility/ax_action_data.h"
......@@ -2503,7 +2504,9 @@ bool OmniboxViewViews::IsURLEligibleForSimplifiedDomainEliding() {
// chrome:, etc.
return (url_scheme == base::UTF8ToUTF16(url::kHttpScheme) ||
url_scheme == base::UTF8ToUTF16(url::kHttpsScheme)) &&
host.is_nonempty();
host.is_nonempty() &&
!net::HostStringIsLocalhost(
base::UTF16ToUTF8(text.substr(host.begin, host.len)));
}
void OmniboxViewViews::ResetToHideOnInteraction() {
......
......@@ -495,7 +495,9 @@ class OmniboxViewViews : public OmniboxView,
// simplified domain. This takes into account the omnibox's current state
// (e.g. the URL shouldn't be elided if the user is currently editing it) as
// well as properties of the current text (e.g. extension URLs or non-URLs
// shouldn't be elided because they may not have simplified domains).
// shouldn't be elided because they may not have simplified domains; localhost
// URLs shouldn't be elided because they are used in development workflows
// where the full URL is useful).
//
// This method does NOT take field trials into account or the "Always show
// full URLs" option. Calling code should check field trial state and
......
......@@ -2675,6 +2675,42 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
gfx::Range(0, kSimplifiedDomainDisplayUrl.size())));
}
// Tests that in the simplified domain field trials, non-http/https and
// localhost URLs are not elided.
TEST_P(OmniboxViewViewsRevealOnHoverTest, UrlsNotEligibleForEliding) {
const base::string16 kTestCases[] = {
// Various URLs that aren't eligible for simplified domain eliding.
base::ASCIIToUTF16("ftp://foo.bar.test/baz"),
base::ASCIIToUTF16("javascript:alert(1)"),
base::ASCIIToUTF16("data:text/html,hello"),
base::ASCIIToUTF16("http://localhost:4000/foo"),
// A smoke test to check that the test code results in
// the URL being elided properly when eligible.
kSimplifiedDomainDisplayUrl,
};
for (const auto& test_case : kTestCases) {
UpdateDisplayURL(test_case);
omnibox_view()->OnThemeChanged();
if (test_case == kSimplifiedDomainDisplayUrl) {
// This case is the smoke test to check that the test setup does properly
// elide URLs that are eligible for eliding.
ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
omnibox_view(), kSimplifiedDomainDisplayUrlScheme,
kSimplifiedDomainDisplayUrlSubdomain,
kSimplifiedDomainDisplayUrlHostnameAndScheme,
kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
} else {
EXPECT_EQ(gfx::ELIDE_TAIL,
omnibox_view()->GetRenderText()->elide_behavior());
EXPECT_EQ(test_case, omnibox_view()->GetRenderText()->GetDisplayText());
EXPECT_EQ(0,
omnibox_view()->GetRenderText()->GetUpdatedDisplayOffset().x());
}
}
}
// Tests that in the hide-on-interaction field trial, when the path changes
// while being elided, the animation is stopped.
TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
......
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