Commit f0f1699f authored by Joel Hockey's avatar Joel Hockey Committed by Commit Bot

Always use default theme for System App popup settings pages

Terminal System App uses an almost black (#101010) theme
by default for the main tabbed window.  However, UX
have requested that the settings page always use white
for the frame, similar to OS settings.

Screenshots:
old: crbug.com/1033339#c7
new: crbug.com/1033339#c19

Since both windows are part of the same app, and it is not
possible to have a manifest with 2 different themes
specified, we will control the theme for
System App TYPE_APP_POPUP to always use default which
is white for light-mode, and something suitable
for dark-mode.

Fix for BrowserNonClientFrameViewMac which had not yet been
updated to support browser TYPE_APP_POPUP as revealed by
the included test.

Bug: 1033339
Change-Id: I7b0d7cc291e5f5895ddfda2ad384256c3519d0a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066378
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743852}
parent 64ff0f4b
...@@ -77,10 +77,11 @@ BrowserNonClientFrameViewMac::BrowserNonClientFrameViewMac( ...@@ -77,10 +77,11 @@ BrowserNonClientFrameViewMac::BrowserNonClientFrameViewMac(
// The window title appears above the web app frame toolbar (if present), // The window title appears above the web app frame toolbar (if present),
// which surrounds the title with minimal-ui buttons on the left, // which surrounds the title with minimal-ui buttons on the left,
// and other controls (such as the app menu button) on the right. // and other controls (such as the app menu button) on the right.
DCHECK(browser_view->ShouldShowWindowTitle()); if (browser_view->ShouldShowWindowTitle()) {
window_title_ = AddChildView( window_title_ = AddChildView(
std::make_unique<views::Label>(browser_view->GetWindowTitle())); std::make_unique<views::Label>(browser_view->GetWindowTitle()));
window_title_->SetID(VIEW_ID_WINDOW_TITLE); window_title_->SetID(VIEW_ID_WINDOW_TITLE);
}
} }
} }
......
...@@ -101,6 +101,16 @@ class AppBrowserControllerBrowserTest : public InProcessBrowserTest { ...@@ -101,6 +101,16 @@ class AppBrowserControllerBrowserTest : public InProcessBrowserTest {
tabbed_app_url_)); tabbed_app_url_));
} }
void InstallAndLaunchMockPopup() {
test_system_web_app_installation_->WaitForAppInstall();
auto params = web_app::CreateSystemWebAppLaunchParams(
browser()->profile(), test_system_web_app_installation_->GetType());
params->disposition = WindowOpenDisposition::NEW_POPUP;
app_browser_ = web_app::LaunchSystemWebApp(
browser()->profile(), test_system_web_app_installation_->GetType(),
test_system_web_app_installation_->GetAppUrl(), *params);
}
GURL GetActiveTabURL() { GURL GetActiveTabURL() {
return app_browser_->tab_strip_model() return app_browser_->tab_strip_model()
->GetActiveWebContents() ->GetActiveWebContents()
...@@ -188,4 +198,10 @@ IN_PROC_BROWSER_TEST_F(AppBrowserControllerBrowserTest, TabLoadNoThemeChange) { ...@@ -188,4 +198,10 @@ IN_PROC_BROWSER_TEST_F(AppBrowserControllerBrowserTest, TabLoadNoThemeChange) {
EXPECT_EQ(GetTabColor(app_browser_), SK_ColorGREEN); EXPECT_EQ(GetTabColor(app_browser_), SK_ColorGREEN);
} }
IN_PROC_BROWSER_TEST_F(AppBrowserControllerBrowserTest,
WhiteThemeForSystemAppPopup) {
InstallAndLaunchMockPopup();
EXPECT_FALSE(app_browser_->app_controller()->GetThemeColor().has_value());
}
} // namespace web_app } // namespace web_app
...@@ -73,6 +73,10 @@ gfx::ImageSkia WebAppBrowserController::GetWindowIcon() const { ...@@ -73,6 +73,10 @@ gfx::ImageSkia WebAppBrowserController::GetWindowIcon() const {
} }
base::Optional<SkColor> WebAppBrowserController::GetThemeColor() const { base::Optional<SkColor> WebAppBrowserController::GetThemeColor() const {
// System App popups (settings pages) always use default theme.
if (is_for_system_web_app() && browser()->is_type_app_popup())
return base::nullopt;
base::Optional<SkColor> web_theme_color = base::Optional<SkColor> web_theme_color =
AppBrowserController::GetThemeColor(); AppBrowserController::GetThemeColor();
if (web_theme_color) if (web_theme_color)
......
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