New issue
Advanced search Search tips

Issue 831769 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner:
Closed: Jan 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

MacViews browser: support "Increase contrast"

Project Member Reported by ellyjo...@chromium.org, Apr 11 2018

Issue description

This needs implementation work at least in:

Tabstrip
Toolbar
Omnibox
Menus

Details: https://support.apple.com/kb/ph25218?locale=en_US
 

Comment 1 by gov...@chromium.org, Apr 25 2018

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.
Labels: -Target-68 Target-69
Description: Show this description
Labels: -M-68 Group-MacOS_Platform_Integration_and_Participation
Labels: M-68
Status: Started (was: Assigned)
<https://chromium-review.googlesource.com/c/chromium/src/+/1142030> is a prototype for top chrome.

Screenshots (all post-change):
"stock" shows the look without "Increase Contrast" (ie, the current look)
"inactive-active-high" shows active & inactive windows with "Increase Contrast" mode
"active-high" shows an active window with active and inactive tabs with "Increase Contrast" mode
"incognito-high" shows an active incognito window with "Increase Contrast" mode
"hovered-high" shows the mouse-glow effect and hover effect on tabs

Things to note:

1) Incognito and active ended up somewhat similar, but the incognito omnibox is black and it has an incognito profile badge. I think that's distinctive enough...
2) The hover effect is "aggressively visible". I'm not sure what I think of it.
stock.png
23.4 KB View Download
inactive-active-high.png
57.6 KB View Download
active-high.png
22.9 KB View Download
incognito-high.png
20.1 KB View Download
hovered-high.png
28.8 KB View Download
Hey Elly - this is looking awesome to me. They really make top Chrome so much more visible. It's a little hard for me to give input on the hover and if it's too aggressive or not, without seeing it live and in action (moving hover from one to another to see how it changes). Will it be possible to take a look at these on my machine soon to play around? 
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 19

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

commit b04cbd7e3f2950dec58f95f6cc78c00148f0c2ce
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Jul 19 13:23:12 2018

mac: add high contrast theme

This change adds a high-contrast theme supplier (IncreasedContrastThemeSupplier)
and enables it on Mac when the system "Increase Contrast" setting is enabled,
or when --force-high-contrast is passed at startup. This theme maximizes
contrast between UI elements.

Bug:  831769 
Change-Id: I6cbb1b67fd95372dd65501feab6639942f767019
Reviewed-on: https://chromium-review.googlesource.com/1142030
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576467}
[modify] https://crrev.com/b04cbd7e3f2950dec58f95f6cc78c00148f0c2ce/chrome/browser/BUILD.gn
[modify] https://crrev.com/b04cbd7e3f2950dec58f95f6cc78c00148f0c2ce/chrome/browser/themes/custom_theme_supplier.h
[add] https://crrev.com/b04cbd7e3f2950dec58f95f6cc78c00148f0c2ce/chrome/browser/themes/increased_contrast_theme_supplier.cc
[add] https://crrev.com/b04cbd7e3f2950dec58f95f6cc78c00148f0c2ce/chrome/browser/themes/increased_contrast_theme_supplier.h
[modify] https://crrev.com/b04cbd7e3f2950dec58f95f6cc78c00148f0c2ce/chrome/browser/themes/theme_service.cc
[modify] https://crrev.com/b04cbd7e3f2950dec58f95f6cc78c00148f0c2ce/chrome/browser/ui/views/location_bar/location_bar_view.cc

#7: Yup - tomorrow's canary should have this support (it just landed in #8).
Invert menus: <https://chromium-review.googlesource.com/c/chromium/src/+/1143445>
Screen Shot 2018-07-19 at 10.29.04 AM.png
69.5 KB View Download
This seems to break when changing Chrome's theme, and when toggling the "Increase contrast" preference.
#11: can you elaborate? "Increase contrast" necessarily won't look good with all themes.
Invert omnibox: <https://chromium-review.googlesource.com/c/chromium/src/+/1143686>
normal.png
73.6 KB View Download
incognito.png
54.2 KB View Download
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 19

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

commit e1e91e62b942b8a2a27ad62dfaee0a6e0cf28b67
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Jul 19 17:42:06 2018

macviews: invert omnibox selection in high contrast

This makes the selected result and its text extremely prominent.

Bug:  831769 
Change-Id: If3456206fe2b6a98cba05caf3a0008cb16b22483
Reviewed-on: https://chromium-review.googlesource.com/1143686
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576553}
[modify] https://crrev.com/e1e91e62b942b8a2a27ad62dfaee0a6e0cf28b67/chrome/browser/ui/omnibox/omnibox_theme.cc
[modify] https://crrev.com/e1e91e62b942b8a2a27ad62dfaee0a6e0cf28b67/chrome/browser/ui/views/omnibox/omnibox_result_view.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 19

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

commit 35dd6e85fdf7266baa2c5406fe0166c5df33859c
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Jul 19 18:40:43 2018

macviews: use invert for high contrast menu selection

This is more Mac-y, not less visible, and considerably prettier than the old
approach of painting an extra inner border stroke.

Bug:  831769 
Change-Id: I1b68b7e1b0e031121de1850709b261331264d3f6
Reviewed-on: https://chromium-review.googlesource.com/1143445
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576570}
[modify] https://crrev.com/35dd6e85fdf7266baa2c5406fe0166c5df33859c/ui/native_theme/native_theme_mac.mm

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 19

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

commit 9c2addd5ba9738eff4d6ea5ad5a4e765c0fe0ee3
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Jul 19 20:15:57 2018

themes: expand --force-high-contrast to other platforms

This lets users of Windows/Linux supply this flag to force the high contrast
appearance. There is no automated high contrast on those platforms yet.

Bug:  831769 
Change-Id: I5dc927857db632999944e3ac8b3ae4b134dfe0be
Reviewed-on: https://chromium-review.googlesource.com/1143695
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576601}
[modify] https://crrev.com/9c2addd5ba9738eff4d6ea5ad5a4e765c0fe0ee3/chrome/browser/themes/theme_service.cc

Yes, I am not commenting on the look in general, but how it reacts to toggling this setting or installing/uninstalling a Chrome theme.

[contrast-toggle.mov] When toggling high contrast, the tab strip does not react. The outline on the omnibox updates when you move the mouse over it, and the menu reacts immediately. Requires a restart to complete.

[theme-toggle.mov] When uninstalling a theme, Chrome switches to regular instead of high contrast.

I can open a new bug if needed; changing the settings is somewhat separate from supporting this feature at all.
contrast-toggle.mov
6.2 MB View Download
theme-toggle.mov
2.0 MB View Download
Ah, yes - please open a new bug about that. We have the same issue with some of the other system appearance settings and a more principled approach is probably in order. Thanks!
fyi: One of the other system preference reports - issue 703781.
Issue 866039.
I am super thrilled with this! The more visible focus indication with keyboard is so helpful (especially in menus), and the darker background tabs make it so much easier to see which tab is in focus. 

A few things:

If I press my profile picture button, then try to arrow down that menu, the focus is the old color (not the darker shade). 

Also, if I press the extension icon for BeyondCorp then tab, the focus ring seems broken on that bottom link. 

In incognito, I would recommend making the shade of the glow on hovered tabs a little bit less bright, to better maintain the contrast between the text and the glowed color. We could just darken it by a few shades -- nothing too drastic. Happy to provide more input on mocks! 

Also, FWIW, the keyboard focus indication is harder to see in incognito compared to non-incognito (when you tab through the toolbar items). Seems to be the older blue?

Thoughts? Thanks! 
Labels: -M-68 M-69
URLs in the omnibox are showing in an unreadable blue text on dark gray background.  Issue 870290 .
Labels: -M-69 -Target-69 Target-70 M-70
Mac triage: The rest of this work has to go to M70.
Cc: pkasting@chromium.org
I'm concerned about this on Windows.  The colors chosen have nothing to do with the system's high contrast colors.  We want to respect those colors, but because the implementation here uses a custom theme provider, it overrides everything platform-specific that e.g. theme_service_win.cc can do (which is where I'm working on trying to add support for the native high contrast colors, and which is the only real place that we can read Windows-specific native constants).

I think the architecture here needs to change to be more platform specific and to not use what amounts to an internal custom theme.  Instead each platform's theme service should respect that platform's high contrast settings, and if necessary we should provide high-contrast fallback colors from GetDefaultColor() in the core theme service code.
#c25: This is the gist of some feedback in code review

https://chromium-review.googlesource.com/c/chromium/src/+/1148462/2/chrome/browser/ui/views/harmony/harmony_typography_provider.cc#80

That cl is still in progress. I think my suggestions will work towards what we want on Windows as well.

Comment 27 Deleted

(For an example of some Windows work this blocks, see https://chromium-review.googlesource.com/c/chromium/src/+/1166547 .)
#25: I can get behind that. On Mac we do probably want the CustomThemeSupplier but we can make that Mac-specific maybe?
I don't know the constraints well enough to advise there -- whether it's possible to get the effects you want on Mac without a custom theme supplier.

Ignoring maintainability for a minute, if you imagined sticking duplicated conditional checks + color returns into every single theme_service_{platform} file to add these sorts of colors, could you do what you want on Mac without a separate theme supplier?  If so, there's probably some way to get what you want (though it's likely not that).

I don't have the bandwidth to do engineering work on this stuff right now, so I leave it to you to think about the stuff above in terms of how you want to architect.
Labels: Hotlist-DesktopUIChecked Hotlist-DesktopUIValid
***Mass UI Triage***
As per dev comments.

Status: WontFix (was: Started)
Closing this bug - it got obsoleted by other work being done by lgrey@.

Sign in to add a comment