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

Issue 831161 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature



Sign in to add a comment

Tint omnibox icons (lock, danger-triangle, etc) to match switcher and menu button color

Project Member Reported by ainslie@chromium.org, Apr 10 2018

Issue description

Following up on the scheme/sub-domain thread and offline discussion:
https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/CwAe-wUS53k

As part of that experiment, let's update the color of the icons in the omnibox always match the switcher and overflow menu icons on the toolbar. This means that in "default" toolbar contexts we'll use the grey for info-i, offline-check, danger-triangle, ssl-lock. And in non-default contexts (incognito, brand-color/theme-color, CCT-app-specified color), we'll tint the icons using our existing contrast ratio logic. 

(note: one special case could be to keep the danger-triangle red in default toolbar contexts. Since you'll have already seen a full-red interstitial with white danger icon,  I'm thinking it's not necessary to use red in the omnibox after that)
 
Status: Assigned (was: Untriaged)
On desktop, the color change probably won't be until M69. I think it's OK to be slightly divergent in case this lands before then.
To comment on #1

If we are using theme colors, we already do not tint the danger icon red.  We only use the red tint if there is no theme color.

Since we are already doing this in existing cases, it seems OK to just expand that to all cases in this new world.
Yup. SGTM. (That's our approach for incognito too, right?)
Correct

The existing approach prior to these changes is here:
https://chromium.googlesource.com/chromium/src/+/5b3d5a34cfaffaed8c6f80d2700b4f34b42c5964/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java#1301

1.) Incognito or dark theme color:
  Use light tint
2.) Using other theme colors:
  Use dark tint
3.) Is dangerous:
  Red
4.) Is secure:
  Green
5.) Everything else:
  Use dark tint

With the new model, we're essentially doing:
1.) Incognito or dark theme color:
  Use light tint
2.) Everything else:
  Use dark tint
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 26 2018

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

commit 6f179c740e9924a67122bd6cfadba4a7b6374405
Author: Ted Choc <tedchoc@google.com>
Date: Thu Apr 26 18:40:46 2018

Move the color state security icon calculation to the data provider.

BUG= 831161 

Change-Id: Ibcc1dc224b25fddb9952f93570390f727931ccde
Reviewed-on: https://chromium-review.googlesource.com/1029195
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554098}
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchBoxDataProvider.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarDataProvider.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModel.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarLayoutTest.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarVoiceRecognitionHandlerTest.java
[modify] https://crrev.com/6f179c740e9924a67122bd6cfadba4a7b6374405/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/ToolbarModelTest.java

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 26 2018

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

commit b91c648171e45a47c0ba370e19e099e833a83621
Author: Ted Choc <tedchoc@google.com>
Date: Thu Apr 26 20:40:07 2018

Remove lock icon colors if in scheme hiding experiment.

BUG= 831161 

Change-Id: Ie9986021a0a7bab7c5410ff3113f2543d400df4a
Reviewed-on: https://chromium-review.googlesource.com/1029206
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554131}
[modify] https://crrev.com/b91c648171e45a47c0ba370e19e099e833a83621/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModel.java

Labels: M-68
Status: Fixed (was: Assigned)

Sign in to add a comment