Commit ef3fa573 authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Fix unreadable title text colour for PWA windows when GTK theme is used

This CL makes OpaqueBrowserFrameView test for whether the user is using
the system theme for the browser before using the hosted app theme
color algorithm.

This CL also updates the foreground color algorithm to always use the
actual frame color to avoid readability bugs like this in general.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=365224&signed_aid=05pRAK75RpeItQNbJQ32zA==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=365225&signed_aid=5PQmY9poCGVOGL8cIt7X3g==&inline=1
Change-Id: Id428054a636b408f59f88c49a0911f6a5052ccc5
Reviewed-on: https://chromium-review.googlesource.com/c/1307032Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604110}
parent b8c0c549
...@@ -639,14 +639,17 @@ bool OpaqueBrowserFrameView::ShouldShowWindowTitleBar() const { ...@@ -639,14 +639,17 @@ bool OpaqueBrowserFrameView::ShouldShowWindowTitleBar() const {
SkColor OpaqueBrowserFrameView::GetReadableFrameForegroundColor( SkColor OpaqueBrowserFrameView::GetReadableFrameForegroundColor(
ActiveState active_state) const { ActiveState active_state) const {
const SkColor frame_color = GetFrameColor(active_state);
if (browser_view()->IsBrowserTypeHostedApp()) { if (browser_view()->IsBrowserTypeHostedApp()) {
base::Optional<SkColor> theme_color = const bool has_site_theme = browser_view()
browser_view()->browser()->hosted_app_controller()->GetThemeColor(); ->browser()
if (theme_color) ->hosted_app_controller()
return color_utils::GetThemedAssetColor(*theme_color); ->GetThemeColor()
.has_value();
if (has_site_theme && !platform_observer_->IsUsingSystemTheme())
return color_utils::GetThemedAssetColor(frame_color);
} }
return color_utils::GetReadableColor(kTitleBarFeatureColor, return color_utils::GetReadableColor(kTitleBarFeatureColor, frame_color);
GetFrameColor(active_state));
} }
void OpaqueBrowserFrameView::PaintRestoredFrameBorder( void OpaqueBrowserFrameView::PaintRestoredFrameBorder(
......
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/extensions/browsertest_util.h" #include "chrome/browser/extensions/browsertest_util.h"
#include "chrome/browser/themes/theme_service.h"
#include "chrome/browser/themes/theme_service_factory.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/hosted_app_button_container.h" #include "chrome/browser/ui/views/frame/hosted_app_button_container.h"
...@@ -83,6 +85,19 @@ IN_PROC_BROWSER_TEST_F(HostedAppOpaqueBrowserFrameViewTest, NoThemeColor) { ...@@ -83,6 +85,19 @@ IN_PROC_BROWSER_TEST_F(HostedAppOpaqueBrowserFrameViewTest, NoThemeColor) {
SK_ColorBLACK); SK_ColorBLACK);
} }
#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
IN_PROC_BROWSER_TEST_F(HostedAppOpaqueBrowserFrameViewTest, SystemThemeColor) {
ThemeService* theme_service =
ThemeServiceFactory::GetForProfile(browser()->profile());
theme_service->UseSystemTheme();
ASSERT_TRUE(theme_service->UsingSystemTheme());
ASSERT_TRUE(InstallAndLaunchHostedApp(SK_ColorBLACK));
EXPECT_EQ(hosted_app_button_container_->active_color_for_testing(),
SK_ColorBLACK);
}
#endif // defined(OS_LINUX) && !defined(OS_CHROMEOS)
IN_PROC_BROWSER_TEST_F(HostedAppOpaqueBrowserFrameViewTest, LightThemeColor) { IN_PROC_BROWSER_TEST_F(HostedAppOpaqueBrowserFrameViewTest, LightThemeColor) {
if (!InstallAndLaunchHostedApp(SK_ColorYELLOW)) if (!InstallAndLaunchHostedApp(SK_ColorYELLOW))
return; return;
......
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