Skip to content

GitLab

T
tangled
Commit 63f67290 authored by Avi Drissman's avatar Avi Drissman Committed by Commit Bot
Browse files
Options

Improve titling of JavaScript alerts. 

If the URL of an alerting page can be unwrapped, do so. This
improves the ability of the user to tell what page is showing
the alert.

BUG=696454
TEST=the dialog in that bug is labeled.

Change-Id: I71358be49418ab6d4a88e04f317241e134e361ce
Reviewed-on: https://chromium-review.googlesource.com/797677
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520787}
parent f3c65a66
Hide whitespace changes
Inline Side-by-side
Showing with 58 additions and 30 deletions
components/app_modal/javascript_dialog_manager.cc
View file @ 63f67290
......@@ -105,20 +105,49 @@ base::string16 JavaScriptDialogManager::GetTitle(
return GetTitleImpl(web_contents->GetURL(), alerting_frame_url);
}
namespace {
// Unwraps an URL to get to an embedded URL.
GURL UnwrapURL(const GURL& url) {
// GURL will unwrap filesystem:// URLs so ask it to do so.
const GURL* unwrapped_url = url.inner_url();
if (unwrapped_url)
return *unwrapped_url;
// GURL::inner_url() should unwrap blob: URLs but doesn't do so
// (https://crbug.com/690091). Therefore, do it manually.
//
// https://url.spec.whatwg.org/#origin defines the origin of a blob:// URL as
// the origin of the URL which results from parsing the "path", which boils
// down to everything after the scheme. GURL's 'GetContent()' gives us exactly
// that. See url::Origin()'s constructor.
if (url.SchemeIsBlob())
return GURL(url.GetContent());
return url;
}
} // namespace
// static
base::string16 JavaScriptDialogManager::GetTitleImpl(
const GURL& parent_frame_url,
const GURL& alerting_frame_url) {
GURL unwrapped_parent_frame_url = UnwrapURL(parent_frame_url);
GURL unwrapped_alerting_frame_url = UnwrapURL(alerting_frame_url);
bool is_same_origin_as_main_frame =
(parent_frame_url.GetOrigin() == alerting_frame_url.GetOrigin());
if (alerting_frame_url.IsStandard() && !alerting_frame_url.SchemeIsFile() &&
!alerting_frame_url.SchemeIsFileSystem()) {
(unwrapped_parent_frame_url.GetOrigin() ==
unwrapped_alerting_frame_url.GetOrigin());
if (unwrapped_alerting_frame_url.IsStandard() &&
!unwrapped_alerting_frame_url.SchemeIsFile()) {
#if defined(OS_ANDROID)
base::string16 url_string = url_formatter::FormatUrlForSecurityDisplay(
alerting_frame_url, url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
unwrapped_alerting_frame_url,
url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
#else
base::string16 url_string = url_formatter::ElideHost(
alerting_frame_url, gfx::FontList(), kUrlElideWidth);
unwrapped_alerting_frame_url, gfx::FontList(), kUrlElideWidth);
#endif
return l10n_util::GetStringFUTF16(
is_same_origin_as_main_frame ? IDS_JAVASCRIPT_MESSAGEBOX_TITLE
......
components/app_modal/javascript_dialog_manager_unittest.cc
View file @ 63f67290
......@@ -32,73 +32,72 @@ TEST(JavaScriptDialogManagerTest, GetTitle) {
"An embedded page at bar.com says", "An embedded page at bar.com says"},
// file:
// -main frame:
// - main frame:
{"file:///path/to/page.html", "file:///path/to/page.html",
"This Page Says", "This page says", "This page says"},
// -subframe:
// - subframe:
{"http://foo.com/", "file:///path/to/page.html",
"An Embedded Page on This Page Says",
"An embedded page on this page says",
"An embedded page on this page says"},
// ftp:
// -main frame:
// - main frame:
{"ftp://foo.com/path/to/page.html", "ftp://foo.com/path/to/page.html",
"foo.com Says", "foo.com says", "ftp://foo.com says"},
// -subframe:
// - subframe:
{"http://foo.com/", "ftp://foo.com/path/to/page.html",
"An Embedded Page at foo.com Says", "An embedded page at foo.com says",
"An embedded page at ftp://foo.com says"},
// data:
// -main frame:
// - main frame:
{"data:blahblah", "data:blahblah", "This Page Says", "This page says",
"This page says"},
// -subframe:
// - subframe:
{"http://foo.com/", "data:blahblah", "An Embedded Page on This Page Says",
"An embedded page on this page says",
"An embedded page on this page says"},
// javascript:
// -main frame:
// - main frame:
{"javascript:abc", "javascript:abc", "This Page Says", "This page says",
"This page says"},
// -subframe:
// - subframe:
{"http://foo.com/", "javascript:abc",
"An Embedded Page on This Page Says",
"An embedded page on this page says",
"An embedded page on this page says"},
// about:
// -main frame:
// - main frame:
{"about:blank", "about:blank", "This Page Says", "This page says",
"This page says"},
// -subframe:
// - subframe:
{"http://foo.com/", "about:blank", "An Embedded Page on This Page Says",
"An embedded page on this page says",
"An embedded page on this page says"},
// blob:
// -main frame:
// - main frame:
{"blob:http://foo.com/66666666-6666-6666-6666-666666666666",
"blob:http://foo.com/66666666-6666-6666-6666-666666666666",
"This Page Says", "This page says", "This page says"},
// -subframe:
{"http://foo.com/",
"foo.com Says", "foo.com says", "foo.com says"},
// - subframe:
{"http://bar.com/",
"blob:http://foo.com/66666666-6666-6666-6666-666666666666",
"An Embedded Page on This Page Says",
"An embedded page on this page says",
"An embedded page on this page says"},
"An Embedded Page at foo.com Says", "An embedded page at foo.com says",
"An embedded page at foo.com says"},
// filesystem:
// -main frame:
{"filesystem:http://foo.com/", "filesystem:http://foo.com/",
"This Page Says", "This page says", "This page says"},
// -subframe:
{"http://foo.com/", "filesystem:http://foo.com/",
"An Embedded Page on This Page Says",
"An embedded page on this page says",
"An embedded page on this page says"},
// - main frame:
{"filesystem:http://foo.com/bar.html",
"filesystem:http://foo.com/bar.html", "foo.com Says", "foo.com says",
"foo.com says"},
// - subframe:
{"http://bar.com/", "filesystem:http://foo.com/bar.html",
"An Embedded Page at foo.com Says", "An embedded page at foo.com says",
"An embedded page at foo.com says"},
};
for (const auto& test_case : cases) {
......
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