New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 856812 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 20
Cc:
Components:
EstimatedDays: 2
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 659451

Blocking:
issue 821996
issue 856820
issue 862664



Sign in to add a comment

Add ReadableBlendTowardContrastingColor(), use for tab titles and other accessibility-sensitive cases

Project Member Reported by pkasting@chromium.org, Jun 26 2018

Issue description

Existing code often does something like:

  return IsDark(bgcolor) ? kGoogleGrey100 : kGoogleGrey800;

This is problematic.  In cases where accessibility mandates a minimum contrast, we generally have to pick color values close to or at the endpoints (white and black/GG900) to ensure we have sufficient contrast in all cases.  Yet in cases where a less-extreme value would have provided sufficient contrast, picking extreme values results in a very high-contrast UI.

For example, the changes on  bug 855091  made the UI higher-contrast in all cases to fix problems that only happened with some background colors, yet the resulting code is still too low-contrast in other cases.

A solution to this is to add a ReadableBlendTowardContrastingColor() function, which works like BlendTowardOppositeLuma() (which I'm imagining would be renamed BlendTowardContrastingColor() as part of  bug 659451 ) but ensures that we blend far enough to ensure a minimum 4.5 contrast ratio.  (This is always achievable.)

So, for example, BlendTowardOppositeLuma(#404040, 0.25) will return #707070, with a contrast ratio of 2.09, but ReadableBlendTowardContrastingColor(#404040, 0.25) will return #ababab, with a contrast ratio of 4.51.

We can then use this for things like tab titles, to do something which matches the design spec by default, but boosts contrast as needed when people change colors.

(API bikeshedding: might need to be able to specify the background you're trying to contrast against separately from the foreground you're alpha-blending; might want to implement by adding a default arg to BlendTowardContrastingColor() that specifies the minimum contrast.)
 
Blocking: 856820
EstimatedDays: 2
Labels: Hotlist-Helper
Labels: Hotlist-Polish
Labels: -Pri-3 Pri-2
Owner: bsep@chromium.org
Status: Assigned (was: Available)
I think we should try to tackle this before launch, and I think if we do it should be done soon.  Raising to P2 and giving to bsep in hopes he can take it.

Bret, you probably want to resolve  bug 659451  first.
Blocking: 862664
Labels: Group-New_Tab_Button
Labels: -Group-New_Tab_Button Group-Tabstrip
Labels: -Pri-2 -Hotlist-Polish Pri-1
Let's go ahead and try to merge this. Ignore branch for this one.
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/33e02b20e8a979088168f63cbb05093093d0665e

commit 33e02b20e8a979088168f63cbb05093093d0665e
Author: Bret Sepulveda <bsep@chromium.org>
Date: Fri Jul 20 21:13:01 2018

Make critical UI text darker when below a11y minimums.

This patch creates a new color_utils function that moves the foreground
color darker or lighter until it meets the minimum contrast ratio
defined by accessibility guidelines. This new function is used for
inactive tab text, the new-tab button, and text on the bookmark bar,
which can all appear on top of user-defined colors.

Bug:  856812 
Change-Id: If0f43da5852056bc0c55c96f79bf20b9c07b9046
Reviewed-on: https://chromium-review.googlesource.com/1141103
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576987}
[modify] https://crrev.com/33e02b20e8a979088168f63cbb05093093d0665e/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
[modify] https://crrev.com/33e02b20e8a979088168f63cbb05093093d0665e/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/33e02b20e8a979088168f63cbb05093093d0665e/ui/gfx/color_utils.cc
[modify] https://crrev.com/33e02b20e8a979088168f63cbb05093093d0665e/ui/gfx/color_utils.h
[modify] https://crrev.com/33e02b20e8a979088168f63cbb05093093d0665e/ui/gfx/color_utils_unittest.cc

Labels: Merge-Request-69
Status: Fixed (was: Assigned)
Split further iteration on the API into bug 866145.
Project Member

Comment 12 by sheriffbot@chromium.org, Jul 21

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please merge your change to M69 branch 3497 by 4:00 PM PT today, so we can pick it up for this week M69 Dev release. Thank you.
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 23

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eec65b3876e5fd96e2e6c31d3208fb8888bda3e5

commit eec65b3876e5fd96e2e6c31d3208fb8888bda3e5
Author: Bret Sepulveda <bsep@chromium.org>
Date: Mon Jul 23 20:05:57 2018

Make critical UI text darker when below a11y minimums.

This patch creates a new color_utils function that moves the foreground
color darker or lighter until it meets the minimum contrast ratio
defined by accessibility guidelines. This new function is used for
inactive tab text, the new-tab button, and text on the bookmark bar,
which can all appear on top of user-defined colors.

TBR=bsep@chromium.org

(cherry picked from commit 33e02b20e8a979088168f63cbb05093093d0665e)

Bug:  856812 
Change-Id: If0f43da5852056bc0c55c96f79bf20b9c07b9046
Reviewed-on: https://chromium-review.googlesource.com/1141103
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#576987}
Reviewed-on: https://chromium-review.googlesource.com/1147323
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#24}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/eec65b3876e5fd96e2e6c31d3208fb8888bda3e5/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
[modify] https://crrev.com/eec65b3876e5fd96e2e6c31d3208fb8888bda3e5/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/eec65b3876e5fd96e2e6c31d3208fb8888bda3e5/ui/gfx/color_utils.cc
[modify] https://crrev.com/eec65b3876e5fd96e2e6c31d3208fb8888bda3e5/ui/gfx/color_utils.h
[modify] https://crrev.com/eec65b3876e5fd96e2e6c31d3208fb8888bda3e5/ui/gfx/color_utils_unittest.cc

Cc: abdulsyed@chromium.org
+abdulsyed@ fyi, M69 merges taken for Proj-MdRefresh .

Sign in to add a comment