Commit 49e83833 authored by Emily Stark's avatar Emily Stark Committed by Chromium LUCI CQ

Disable simplified domain elision for blob, filesystem, view-source URLs

Simplified domain display was supposed to be disabled for
non-https/https URLs. However, the code was previously using a parsing
function that "hid" these schemes by actually parsing the inner URL
when one of these schemes is in use. This CL fixes this by using
Parse() instead of ParseForEmphasizeComponents(), and adds a test.

Bug: 1147900
Change-Id: Id2535286768eb264c074ae90371f52aef3f6d01f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2607669Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839815}
parent 843d8de5
......@@ -2584,27 +2584,33 @@ bool OmniboxViewViews::IsURLEligibleForSimplifiedDomainEliding() {
if (!model()->CurrentTextIsURL())
return false;
base::string16 text = GetText();
url::Component scheme, host;
AutocompleteInput::ParseForEmphasizeComponents(
text, model()->client()->GetSchemeClassifier(), &scheme, &host);
url::Parsed parts;
base::string16 scheme_str;
// Call Parse() here instead of ParseForEmphasizeComponents() because the
// latter parses the inner URL for blob:, filesystem:, and view-source: URLs.
// For those schemes, we want the outer scheme so that we can disable elision
// for those schemes.
AutocompleteInput::Parse(text, std::string(),
model()->client()->GetSchemeClassifier(), &parts,
&scheme_str, nullptr);
// TODO(crbug.com/1117631): Simplified domain elision can have bugs for some
// URLs with bidirectional hosts, disable elision for those URLs while the
// bugs are fixed.
const base::string16 url_host = text.substr(host.begin, host.len);
const base::string16 url_host = text.substr(parts.host.begin, parts.host.len);
if (base::i18n::GetStringDirection(url_host) ==
base::i18n::TextDirection::UNKNOWN_DIRECTION) {
return false;
}
const base::string16 url_scheme = text.substr(scheme.begin, scheme.len);
// Simplified domain display only makes sense for http/https schemes; for now
// we don't want to mess with the display of other URLs like data:, blob:,
// chrome:, etc.
return (url_scheme == base::UTF8ToUTF16(url::kHttpScheme) ||
url_scheme == base::UTF8ToUTF16(url::kHttpsScheme)) &&
host.is_nonempty() &&
return (scheme_str == base::UTF8ToUTF16(url::kHttpScheme) ||
scheme_str == base::UTF8ToUTF16(url::kHttpsScheme)) &&
!url_host.empty() &&
!net::HostStringIsLocalhost(
base::UTF16ToUTF8(text.substr(host.begin, host.len)));
base::UTF16ToUTF8(text.substr(parts.host.begin, parts.host.len)));
}
void OmniboxViewViews::ResetToHideOnInteraction() {
......
......@@ -2842,6 +2842,9 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, UrlsNotEligibleForEliding) {
base::ASCIIToUTF16("javascript:alert(1)"),
base::ASCIIToUTF16("data:text/html,hello"),
base::ASCIIToUTF16("http://localhost:4000/foo"),
base::ASCIIToUTF16("blob:https://example.test/"),
base::ASCIIToUTF16("view-source:https://example.test/"),
base::ASCIIToUTF16("filesystem:https://example.test/a"),
// A smoke test to check that the test code results in
// the URL being elided properly when eligible.
kSimplifiedDomainDisplayUrl,
......
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