Commit 07d56218 authored by Katie D's avatar Katie D Committed by Commit Bot

Updates url display behavior on Reader Mode pages to be like original pages.

* No HTTP or HTTPS in steady state
* Editing returns HTTPS into view (but does not restore HTTP)
* Focusing and hitting enter goes to original URL without downgrading
  HTTPS users

This makes the behavior of reader mode page URLs in the omnibox
appear the same as the behavior on their original pages.

Bug: 992073
Change-Id: Ic8f8f75545e0aaf3562190908f8136c7b04351eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144617
Commit-Queue: Katie Dektar <katie@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758588}
parent 8dc3a74c
......@@ -70,6 +70,16 @@ base::string16 LocationBarModelImpl::GetURLForDisplay() const {
if (base::FeatureList::IsEnabled(omnibox::kHideFileUrlScheme))
format_types |= url_formatter::kFormatUrlOmitFileScheme;
if (dom_distiller::url_utils::IsDistilledPage(GetURL())) {
// We explicitly elide the scheme here to ensure that HTTPS and HTTP will
// be removed for display: Reader mode pages should not display a scheme,
// and should only run on HTTP/HTTPS pages.
// Users will be able to see the scheme when the URL is focused or being
// edited in the omnibox.
format_types |= url_formatter::kFormatUrlOmitHTTP;
format_types |= url_formatter::kFormatUrlOmitHTTPS;
}
return GetFormattedURL(format_types);
}
......@@ -87,10 +97,10 @@ base::string16 LocationBarModelImpl::GetFormattedURL(
GURL url(GetURL());
// Special handling for dom-distiller:. Instead of showing internal reader
// mode URLs, show the original article URL without a http or https scheme.
// Note that this does not disallow the user from copy-pasting the reader
// mode URL, or from seeing it in the view-source url. Note that this also
// impacts GetFormattedFullURL which uses GetFormattedURL as a helper.
// mode URLs, show the original article URL in the omnibox.
// Note that this does not disallow the user from seeing the distilled page
// URL in the view-source url or devtools. Note that this also impacts
// GetFormattedFullURL which uses GetFormattedURL as a helper.
// Virtual URLs were not a good solution for Reader Mode URLs because some
// security UI is based off of the virtual URL rather than the original URL,
// and Reader Mode has its own security chip. In addition virtual URLs would
......@@ -98,13 +108,8 @@ base::string16 LocationBarModelImpl::GetFormattedURL(
// Reader Mode pages.
// Note: if the URL begins with dom-distiller:// but is invalid we display it
// as-is because it cannot be transformed into an article URL.
if (dom_distiller::url_utils::IsDistilledPage(url)) {
// Ensure that HTTPS and HTTP will be removed. Reader mode should not
// display a scheme, and should only run on HTTP/HTTPS pages.
format_types |= url_formatter::kFormatUrlOmitHTTP;
format_types |= url_formatter::kFormatUrlOmitHTTPS;
if (dom_distiller::url_utils::IsDistilledPage(url))
url = dom_distiller::url_utils::GetOriginalUrlFromDistillerUrl(url);
}
// Note that we can't unescape spaces here, because if the user copies this
// and pastes it into another program, that program may think the URL ends at
......
......@@ -149,13 +149,16 @@ TEST_F(LocationBarModelImplTest, FormatsReaderModeUrls) {
EXPECT_EQ(originalDisplayUrl, model()->GetURLForDisplay());
EXPECT_EQ(originalFormattedFullUrl, model()->GetFormattedFullURL());
// Similarly, https scheme should also be hidden.
// Similarly, https scheme should also be hidden, except from
// GetFormattedFullURL, because kFormatUrlOmitDefaults does not omit https.
const GURL https_url("https://www.example.com/article.html");
distilled = dom_distiller::url_utils::GetDistillerViewUrlFromUrl(
dom_distiller::kDomDistillerScheme, https_url, "title");
delegate()->SetURL(distilled);
EXPECT_EQ(originalDisplayUrl, model()->GetURLForDisplay());
EXPECT_EQ(originalFormattedFullUrl, model()->GetFormattedFullURL());
EXPECT_EQ(
base::ASCIIToUTF16("https://www.example.com/article.html/TestSuffix"),
model()->GetFormattedFullURL());
// Invalid dom-distiller:// URLs should be shown, because they do not
// correspond to any article.
......
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