New issue
Advanced search Search tips

Issue 863474 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Update URL text colors

Project Member Reported by bettes@chromium.org, Jul 13

Issue description

Protocols and paths are too dark in Refresh, for both default and incognito

----

Default

What is the expected result?
Protocol: GG600 (#80868B) 
Path / parameters / fragments / etc: GG600 (#80868B) 


What happens instead?
Protocol: ~#5F6367
Path, Parameters, fragments, etc: ~#5F6367  

----

Incognito

What is the expected result?
Protocol: GG500 (#9AA0A6) 
Path / parameters / fragments / etc: GG500 (#9AA0A6) 


What happens instead?
Protocol: ~#BDC1C5
Path, Parameters, fragments, etc: ~#BDC1C5  


----

Subdomain/domain colors are WAI in both modes 

Default, Subdomain / Domain: GG800 
Incognito, Subdomain / Domain: GG100 (#F1F3F4) 

----

All contrast levels are equal to or better than pre-refresh. 

 
Cc: jdonnelly@chromium.org
Owner: manukh@chromium.org
Status: Assigned (was: Untriaged)
manukh: when you're ready to tackle this, let me know and I can point you at where these are defined.
Labels: Group-Omnibox
Owner: orinj@chromium.org
orinj: There's special coloring for the protocol text and path text in the steady-state omnibox. I think these values are defined in OmniboxTheme and may just need a conditional change when IsRefresh() is true. Or maybe there's something more complicated going on.
Yeah, what's happening is:

This line sets the URL components to LOCATION_BAR_TEXT_DIMMED:
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/omnibox/omnibox_view_views.cc?rcl=c466f89410b80d4fe5aad66f7de507138c6a0416&l=744

And this is currently yielding GG700 standard or GG400 when dark, but the bug suggests the spec is GG600 (or GG500 when dark).
https://cs.chromium.org/chromium/src/chrome/browser/ui/omnibox/omnibox_theme.cc?rcl=bb2cc0c4547aedb7ee97d32916980849b7523ec7&l=255

Since the top of this function has a guard:
  if (!ui::MaterialDesignController::IsNewerMaterialUi())
    return GetLegacyColor(part, tint, state);
I think I can just change these values directly and it will only affect Refresh.  Will submit a CL now.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 18

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

commit 4f199c019577fb8fc1f39edc0254ff3886481a30
Author: Orin Jaworski <orinj@chromium.org>
Date: Wed Jul 18 02:48:38 2018

[omnibox] Lighten dimmed URL text in Refresh to GG600 per spec

The URL path, parameters, etc. (LOCATION_BAR_TEXT_DIMMED)
were being lightened to GG700 but now get GG600 per spec.
The RESULTS_ICON and RESULTS_TEXT_DIMMED using the same
colors remain intact.

Bug:  863474 
Change-Id: I1dc2efb763642d88cbc27d832b0411a10c3310c8
Reviewed-on: https://chromium-review.googlesource.com/1141294
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575916}
[modify] https://crrev.com/4f199c019577fb8fc1f39edc0254ff3886481a30/chrome/browser/ui/omnibox/omnibox_theme.cc

Cc: orinj@chromium.org
Labels: -Pri-1 Pri-3
Owner: bettes@chromium.org
I believe this is fixed in today's Canary. Assigning back to bettes to verify.
Status: Fixed (was: Assigned)
Verified

Sign in to add a comment