New issue
Advanced search Search tips

Issue 853637 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : 'Import bookmarks now...' text color doesn't appears as expected

Reported by rp...@etouch.net, Jun 18 2018

Issue description

Version: 69.0.3464.0 (Official Build)Revision 3c26b60e3842fee660bcff5eb35aa0587d795f02-refs/branch-heads/3464@{#1}(32/64-bit)
OS: Windows (7,8,8.1,10),Linux(14.04 LTS)OS

What steps will reproduce the problem?
1. Launch chrome,navigate to NTP and observe 'Import bookmarks now...' text on Bookmark bar
 
Actual: 'Import bookmarks now...' text color doesn't appears as expected
Expected: 'Import bookmarks now...' text color should be seen properly i.e Blue in color

This is regression issue, broken in ‘M 69’ and will soon update other info :
Good build: 69.0.3457.0  (Revision: 566679).
Bad build: 69.0.3460.0 (Revision: 567312).

Note : Issue is not seen on Mac OS.

 
Actual_screenshot.png
105 KB View Download
Expected_screenshot.png
58.0 KB View Download

Comment 1 by rp...@etouch.net, Jun 18 2018

Labels: hasbisect
Owner: ftirelo@chromium.org
Status: Assigned (was: Unconfirmed)
You are probably looking for a change made after 567151 (known good), but no later than 567152 (first known bad).

Narrow Bisect :
  https://chromium.googlesource.com/chromium/src/+log/c80df7c4d5f048c28e6c4c99ee2c8c9ad98acaf9..dbbd9b766e3780f41257196537625c4e668bda1c

Suspecting: r567152 from Narrow bisect

@ftirelo: Could you please help to reassign if your change is not the cause for this change.

Note:Unable to provide bisect using per-revision script,Hence providing bisect with old script.

Cc: rfeng@chromium.org bettes@chromium.org est...@chromium.org msw@chromium.org
Thanks for the report. Indeed, the regression was introduced by the CL. The reason for the regression is that the link color is only respected if contrast ratio is less than 4.5 (value taken from w3c accessibility guidelines); otherwise, it uses the text color: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/bookmarks/bookmark_bar_instructions_view.cc?type=cs&l=133-143

Please notice that this is the only place where constant kMinimumReadableContrastRatio is used.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 20 2018

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

commit 5b987322c193d6c1f2ef1fa4e98a9d055fa6d48c
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Wed Jun 20 15:06:48 2018

Revert "[Views] Adjust secondary info and link style colors for MD2"

This reverts commit dbbd9b766e3780f41257196537625c4e668bda1c.

Reason for revert: contrast ratio for links is below 4.5:1 threshold

Original change's description:
> [Views] Adjust secondary info and link style colors for MD2
>
> Source: https://docs.google.com/presentation/d/1EO7TOpIMJ7QHjaTVw9St-q6naKwtXX2TwzMirG5EsKY/edit#slide=id.g399d5e0d81_22_0
>
> Bug:  852094 
> Change-Id: I0bbfc28d214bed310fdb4a23cd804442d5751590
> Reviewed-on: https://chromium-review.googlesource.com/1099560
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Evan Stade <estade@chromium.org>
> Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#567152}

TBR=msw@chromium.org,estade@chromium.org,ftirelo@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  852094 ,  853637 
Change-Id: I169e1f39d120c714e37c0534115d692a499c591f
Reviewed-on: https://chromium-review.googlesource.com/1106457
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568845}
[modify] https://crrev.com/5b987322c193d6c1f2ef1fa4e98a9d055fa6d48c/chrome/browser/ui/views/harmony/harmony_typography_provider.cc
[modify] https://crrev.com/5b987322c193d6c1f2ef1fa4e98a9d055fa6d48c/ui/native_theme/common_theme.cc

Status: Fixed (was: Assigned)
Reverted the CL that caused the regression. We will need to figure out next what to do with link colors though.

According to MD2, links are supposed to be GB 600. However, on the bookmarks bar background, contrast is not enough. Maybe, we will need to update that background as well.

Sign in to add a comment