Commit 003df7b7 authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Support unicode characters for origins in hosted app UI

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=328381&signed_aid=VY4yoX2RjWAXH9GdA_fOqA==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=328382&signed_aid=xZ3u2qp0ckn4lR26-iNcmA==&inline=1

Bug: 820314
Change-Id: I8b2eb00c392e11578722656cee876db7abf49640
Reviewed-on: https://chromium-review.googlesource.com/956364
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Reviewed-by: default avatarMatt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542966}
parent 77d0c23d
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "chrome/common/extensions/manifest_handlers/app_launch_info.h" #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
#include "chrome/common/extensions/manifest_handlers/app_theme_color_info.h" #include "chrome/common/extensions/manifest_handlers/app_theme_color_info.h"
#include "components/security_state/core/security_state.h" #include "components/security_state/core/security_state.h"
#include "components/url_formatter/url_formatter.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
...@@ -79,6 +80,18 @@ bool HostedAppBrowserController::IsForExperimentalHostedAppBrowser( ...@@ -79,6 +80,18 @@ bool HostedAppBrowserController::IsForExperimentalHostedAppBrowser(
IsForHostedApp(browser); IsForHostedApp(browser);
} }
// static
base::string16 HostedAppBrowserController::FormatUrlOrigin(const GURL& url) {
return url_formatter::FormatUrl(
url.GetOrigin(),
url_formatter::kFormatUrlOmitUsernamePassword |
url_formatter::kFormatUrlOmitHTTPS |
url_formatter::kFormatUrlOmitHTTP |
url_formatter::kFormatUrlOmitTrailingSlashOnBareHostname |
url_formatter::kFormatUrlOmitTrivialSubdomains,
net::UnescapeRule::SPACES, nullptr, nullptr, nullptr);
}
HostedAppBrowserController::HostedAppBrowserController(Browser* browser) HostedAppBrowserController::HostedAppBrowserController(Browser* browser)
: SiteEngagementObserver(SiteEngagementService::Get(browser->profile())), : SiteEngagementObserver(SiteEngagementService::Get(browser->profile())),
browser_(browser), browser_(browser),
...@@ -199,8 +212,8 @@ std::string HostedAppBrowserController::GetAppShortName() const { ...@@ -199,8 +212,8 @@ std::string HostedAppBrowserController::GetAppShortName() const {
return GetExtension()->short_name(); return GetExtension()->short_name();
} }
url::Origin HostedAppBrowserController::GetUrlOrigin() const { base::string16 HostedAppBrowserController::GetFormattedUrlOrigin() const {
return url::Origin::Create(AppLaunchInfo::GetLaunchWebURL(GetExtension())); return FormatUrlOrigin(AppLaunchInfo::GetLaunchWebURL(GetExtension()));
} }
void HostedAppBrowserController::OnEngagementEvent( void HostedAppBrowserController::OnEngagementEvent(
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/strings/string16.h"
#include "chrome/browser/engagement/site_engagement_observer.h" #include "chrome/browser/engagement/site_engagement_observer.h"
#include "third_party/skia/include/core/SkColor.h" #include "third_party/skia/include/core/SkColor.h"
...@@ -33,6 +34,9 @@ class HostedAppBrowserController : public SiteEngagementObserver { ...@@ -33,6 +34,9 @@ class HostedAppBrowserController : public SiteEngagementObserver {
// Returns whether |browser| uses the experimental hosted app experience. // Returns whether |browser| uses the experimental hosted app experience.
static bool IsForExperimentalHostedAppBrowser(const Browser* browser); static bool IsForExperimentalHostedAppBrowser(const Browser* browser);
// Renders |url|'s origin as Unicode.
static base::string16 FormatUrlOrigin(const GURL& url);
explicit HostedAppBrowserController(Browser* browser); explicit HostedAppBrowserController(Browser* browser);
~HostedAppBrowserController() override; ~HostedAppBrowserController() override;
...@@ -63,8 +67,9 @@ class HostedAppBrowserController : public SiteEngagementObserver { ...@@ -63,8 +67,9 @@ class HostedAppBrowserController : public SiteEngagementObserver {
// Gets the short name of the app. // Gets the short name of the app.
std::string GetAppShortName() const; std::string GetAppShortName() const;
// Gets the origin of the app start url (e.g www.example.com.au). // Gets the origin of the app start url suitable for display (e.g
url::Origin GetUrlOrigin() const; // example.com.au).
base::string16 GetFormattedUrlOrigin() const;
// Gets the extension for this controller. // Gets the extension for this controller.
const Extension* GetExtension() const; const Extension* GetExtension() const;
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
#include "base/strings/utf_string_conversions.h"
#include "testing/gtest/include/gtest/gtest.h"
// Tests that various URLs get formatted as an origin string in the desired way.
TEST(HostedAppBrowserController, FormatUrlOrigin) {
constexpr struct {
const char* const input;
const wchar_t* const expectation;
} kTestCases[] = {
// www and trailing / are dropped.
{"https://www.test.com/", L"test.com"},
// Non-origin components are dropped.
{"https://www.test.com/with/path/to/file.html", L"test.com"},
// http:// is dropped.
{"http://www.insecure.com/", L"insecure.com"},
// Subdomains other than www are preserved.
{"https://subdomain.domain.com/", L"subdomain.domain.com"},
// Punycode gets rendered as Unicode.
{"https://xn--1lq90ic7f1rc.cn", L"\x5317\x4eac\x5927\x5b78.cn"},
// Check that the block list is being applied, taken from more
// comprehensive tests in
// components/url_formatter/url_formatter_unittest.cc.
{"https://xn--36c-tfa.com", L"xn--36c-tfa.com"},
};
for (auto test_case : kTestCases) {
EXPECT_EQ(extensions::HostedAppBrowserController::FormatUrlOrigin(
GURL(test_case.input)),
base::WideToUTF16(test_case.expectation));
}
}
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "chrome/browser/media/router/media_router_feature.h" #include "chrome/browser/media/router/media_router_feature.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
#include "chrome/grit/chromium_strings.h" #include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
...@@ -27,13 +28,12 @@ void HostedAppMenuModel::Build() { ...@@ -27,13 +28,12 @@ void HostedAppMenuModel::Build() {
AddItemWithStringId(IDC_HOSTED_APP_MENU_APP_INFO, AddItemWithStringId(IDC_HOSTED_APP_MENU_APP_INFO,
IDS_APP_CONTEXT_MENU_SHOW_INFO); IDS_APP_CONTEXT_MENU_SHOW_INFO);
int app_info_index = GetItemCount() - 1; int app_info_index = GetItemCount() - 1;
SetMinorText( SetMinorText(app_info_index,
app_info_index, extensions::HostedAppBrowserController::FormatUrlOrigin(
base::UTF8ToUTF16(url::Origin::Create(browser() browser()
->tab_strip_model() ->tab_strip_model()
->GetActiveWebContents() ->GetActiveWebContents()
->GetVisibleURL()) ->GetVisibleURL()));
.host()));
SetMinorIcon(app_info_index, browser()->toolbar_model()->GetVectorIcon()); SetMinorIcon(app_info_index, browser()->toolbar_model()->GetVectorIcon());
AddSeparator(ui::NORMAL_SEPARATOR); AddSeparator(ui::NORMAL_SEPARATOR);
......
...@@ -623,8 +623,7 @@ BrowserNonClientFrameViewAsh::CreateFrameHeader() { ...@@ -623,8 +623,7 @@ BrowserNonClientFrameViewAsh::CreateFrameHeader() {
// Add the origin text. // Add the origin text.
frame_header_origin_text_ = frame_header_origin_text_ =
std::make_unique<ash::FrameHeaderOriginText>( std::make_unique<ash::FrameHeaderOriginText>(
base::UTF8ToUTF16( browser->hosted_app_controller()->GetFormattedUrlOrigin(),
browser->hosted_app_controller()->GetUrlOrigin().host()),
active_color, inactive_color, active_color, inactive_color,
default_frame_header->GetActiveFrameColor(), default_frame_header->GetActiveFrameColor(),
default_frame_header->GetInactiveFrameColor()) default_frame_header->GetInactiveFrameColor())
......
...@@ -2944,6 +2944,7 @@ test("unit_tests") { ...@@ -2944,6 +2944,7 @@ test("unit_tests") {
"../browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc", "../browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc",
"../browser/ui/extensions/extension_action_view_controller_unittest.cc", "../browser/ui/extensions/extension_action_view_controller_unittest.cc",
"../browser/ui/extensions/extension_message_bubble_bridge_unittest.cc", "../browser/ui/extensions/extension_message_bubble_bridge_unittest.cc",
"../browser/ui/extensions/hosted_app_browser_controller_unittest.cc",
"../browser/ui/global_error/global_error_service_unittest.cc", "../browser/ui/global_error/global_error_service_unittest.cc",
"../browser/ui/omnibox/chrome_omnibox_navigation_observer_unittest.cc", "../browser/ui/omnibox/chrome_omnibox_navigation_observer_unittest.cc",
"../browser/ui/omnibox/clipboard_utils_unittest.cc", "../browser/ui/omnibox/clipboard_utils_unittest.cc",
......
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