Commit a353d4cd authored by Tom Anderson's avatar Tom Anderson Committed by Commit Bot

Ensure omnibox foreground colors are readable over hovered backgrounds

R=pkasting
BUG=None

Change-Id: Ife6d481a840fc23b7aac527de332e4185d744ed8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1822841
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699472}
parent c4db4404
...@@ -1181,8 +1181,12 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor( ...@@ -1181,8 +1181,12 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor(
const bool dark = IsDark(bg.value); const bool dark = IsDark(bg.value);
auto results_bg_color = [&]() { return get_color_with_max_contrast(fg); }; auto results_bg_color = [&]() { return get_color_with_max_contrast(fg); };
auto bg_hovered_color = [&]() { return blend_toward_max_contrast(bg, 0x0A); };
auto security_chip_color = [&](OmniboxColor color) { auto security_chip_color = [&](OmniboxColor color) {
return blend_for_min_contrast(color, bg); return blend_for_min_contrast(color, bg_hovered_color());
};
auto results_bg_hovered_color = [&]() {
return blend_toward_max_contrast(results_bg_color(), 0x1A);
}; };
auto url_color = [&](OmniboxColor bg) { auto url_color = [&](OmniboxColor bg) {
return blend_for_min_contrast( return blend_for_min_contrast(
...@@ -1204,7 +1208,7 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor( ...@@ -1204,7 +1208,7 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor(
case TP::COLOR_OMNIBOX_BACKGROUND: case TP::COLOR_OMNIBOX_BACKGROUND:
return bg; return bg;
case TP::COLOR_OMNIBOX_BACKGROUND_HOVERED: case TP::COLOR_OMNIBOX_BACKGROUND_HOVERED:
return blend_toward_max_contrast(bg, 0x0A); return bg_hovered_color();
case TP::COLOR_OMNIBOX_RESULTS_BG: case TP::COLOR_OMNIBOX_RESULTS_BG:
return results_bg_color(); return results_bg_color();
case TP::COLOR_OMNIBOX_RESULTS_BG_SELECTED: case TP::COLOR_OMNIBOX_RESULTS_BG_SELECTED:
...@@ -1220,9 +1224,9 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor( ...@@ -1220,9 +1224,9 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor(
case TP::COLOR_OMNIBOX_BUBBLE_OUTLINE_EXPERIMENTAL_KEYWORD_MODE: case TP::COLOR_OMNIBOX_BUBBLE_OUTLINE_EXPERIMENTAL_KEYWORD_MODE:
return url_color(results_bg_color()); return url_color(results_bg_color());
case TP::COLOR_OMNIBOX_TEXT_DIMMED: case TP::COLOR_OMNIBOX_TEXT_DIMMED:
return blend_with_clamped_contrast(bg); return blend_with_clamped_contrast(bg_hovered_color());
case TP::COLOR_OMNIBOX_RESULTS_TEXT_DIMMED: case TP::COLOR_OMNIBOX_RESULTS_TEXT_DIMMED:
return blend_with_clamped_contrast(results_bg_color()); return blend_with_clamped_contrast(results_bg_hovered_color());
case TP::COLOR_OMNIBOX_RESULTS_TEXT_DIMMED_SELECTED: case TP::COLOR_OMNIBOX_RESULTS_TEXT_DIMMED_SELECTED:
return blend_with_clamped_contrast(results_bg_selected_color()); return blend_with_clamped_contrast(results_bg_selected_color());
case TP::COLOR_OMNIBOX_RESULTS_ICON: case TP::COLOR_OMNIBOX_RESULTS_ICON:
...@@ -1232,9 +1236,9 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor( ...@@ -1232,9 +1236,9 @@ base::Optional<SkColor> ThemeService::GetOmniboxColor(
return blend_for_min_contrast(derive_default_icon_color(fg), return blend_for_min_contrast(derive_default_icon_color(fg),
results_bg_selected_color()); results_bg_selected_color());
case TP::COLOR_OMNIBOX_RESULTS_BG_HOVERED: case TP::COLOR_OMNIBOX_RESULTS_BG_HOVERED:
return blend_toward_max_contrast(results_bg_color(), 0x1A); return results_bg_hovered_color();
case TP::COLOR_OMNIBOX_RESULTS_URL: case TP::COLOR_OMNIBOX_RESULTS_URL:
return url_color(results_bg_color()); return url_color(results_bg_hovered_color());
case TP::COLOR_OMNIBOX_RESULTS_URL_SELECTED: case TP::COLOR_OMNIBOX_RESULTS_URL_SELECTED:
return url_color(results_bg_selected_color()); return url_color(results_bg_selected_color());
case TP::COLOR_OMNIBOX_SECURITY_CHIP_DEFAULT: case TP::COLOR_OMNIBOX_SECURITY_CHIP_DEFAULT:
......
...@@ -586,19 +586,12 @@ TEST_F(ThemeServiceTest, OmniboxContrast) { ...@@ -586,19 +586,12 @@ TEST_F(ThemeServiceTest, OmniboxContrast) {
TP::COLOR_OMNIBOX_BACKGROUND}, TP::COLOR_OMNIBOX_BACKGROUND},
{TP::COLOR_OMNIBOX_SECURITY_CHIP_DANGEROUS, {TP::COLOR_OMNIBOX_SECURITY_CHIP_DANGEROUS,
TP::COLOR_OMNIBOX_RESULTS_BG}, TP::COLOR_OMNIBOX_RESULTS_BG},
// TODO(thomasanderson): Because colors are computed relative to {TP::COLOR_OMNIBOX_TEXT_DIMMED, TP::COLOR_OMNIBOX_BACKGROUND_HOVERED},
// non-hovered backgrounds, some colors over hovered backgrounds do {TP::COLOR_OMNIBOX_RESULTS_TEXT_DIMMED,
// not have sufficient contrast in all configurations. Computing the TP::COLOR_OMNIBOX_RESULTS_BG_HOVERED},
// non-contrasty colors here relative to hovered backgrounds should {TP::COLOR_OMNIBOX_RESULTS_URL, TP::COLOR_OMNIBOX_RESULTS_BG_HOVERED},
// fix this and not reduce contrast of non-hovered backgrounds. {TP::COLOR_OMNIBOX_SECURITY_CHIP_DANGEROUS,
// {TP::COLOR_OMNIBOX_TEXT_DIMMED, TP::COLOR_OMNIBOX_BACKGROUND_HOVERED},
// TP::COLOR_OMNIBOX_BACKGROUND_HOVERED},
// {TP::COLOR_OMNIBOX_RESULTS_TEXT_DIMMED,
// TP::COLOR_OMNIBOX_RESULTS_BG_HOVERED},
// {TP::COLOR_OMNIBOX_RESULTS_URL,
// TP::COLOR_OMNIBOX_RESULTS_BG_HOVERED},
// {TP::COLOR_OMNIBOX_SECURITY_CHIP_DANGEROUS,
// TP::COLOR_OMNIBOX_BACKGROUND_HOVERED},
}; };
auto check_sufficient_contrast = [&](int id1, int id2) { auto check_sufficient_contrast = [&](int id1, int id2) {
const float contrast = color_utils::GetContrastRatio( const float contrast = color_utils::GetContrastRatio(
......
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