Assigning to bettes for spec.

From  Issue 863094 :
- window controls dimmed
- active tabstrip blended 70% against white or hex color: #E7EAED

In addition to:
- toolbar icons: 28% (same as disabled state)
- URL: 50%
- Tabs and NTB: 50%

https://gallery.googleplex.com/projects/MCHbtQVoQ2HCZW9TjXpqhOa9/files/MCEhmyWmAmKVJz7eZktUlsQ2

Deleted: v2 - default (MacOS).png 309 KB

	v2 - default (MacOS).png 309 KB View Download

Deleted: v2 - default (Windows).png 298 KB

	v2 - default (Windows).png 298 KB View Download

Deleted: v2 - themes.png 531 KB

	v2 - themes.png 531 KB View Download

Blending things other than the background tabs/window controls seems going a bit far -- not only is it not native Windows behavior but it can lower contrast (already a problem on the stuff that does get blended) and reduce legibility/usability until you focus the window.

I don't think the UX of Mac and Window conventions are terribly different here, but if we feel that following conventions is important than we break up our approach for each. 

Echoing c5, we can just dim the tabstrip and window controls for Windows and CrOS.

https://gallery.googleplex.com/projects/MCHbtQVoQ2HCZW9TjXpqhOa9/files/MCGzOAHDM-CKDrHDLgts3J1swMSJbxAVNpw

Deleted: v2 - default WIN.png 290 KB

	v2 - default WIN.png 290 KB View Download

Mac inactive window treatment: crbug.com/848593

That seems better to me for Windows.  I think Windows might blend with alpha 0.4 instead of 0.5 (so even more faded), but I only tested my custom color combo and not the defaults or a range of other custom cases.

If someone implements this who's really detail-oriented, they could try to match the OS more precisely.

Upgrading to P2 based off of design Post M69 triage.

dfried@ had a good point about dimming: the active tab is the same color as the toolbar, so if we dim the tab strip like this, the active tab will suddenly be dimmer than the toolbar. This doesn't seem desirable. Any recommendations?

My hope was to only dim the controls (text, icons) and not the background colors/images.  You don't really want to muck with those background things anyway since their colors are already adjusted to have the shades we (or theme authors) want.

So it looks like the code for picking tab colors already looks at whether the window is active or inactive; see BrowserNonClientFrameView::GetTab{Foreground,Background}Color(). For the tab text and tab buttons, I think we'd just need to update the default COLOR_BACKGROUND_TAB{,_TEXT}_INACTIVE colors. If we want to change the text color in the active tab as well, we could add COLOR_TAB_TEXT_INACTIVE.

This will also update the tab separators and the new tab button. The tab separator color is computed by blending the text color with the tab background color (https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/tab_strip.cc?rcl=4184918e72d3095810e9bfdd73666f6580fe10d6&l=2146) and the new tab button uses the tab background color (https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/new_tab_button.cc?rcl=4184918e72d3095810e9bfdd73666f6580fe10d6&l=532).

Hopefully we'll update the close icons too, but we may have to reduce the contrast in those case or something.

After looking further, it looks like BrowserNonClientFrameView::GetTabForegroundColor() actually treats the refresh UI as a special case and computes the colors for inactive tabs based on hard-coded constants and the background color, taking into account the minimum contrast using color_utils::GetColorWithMinimumContrast(). The new tab button and tab close button appear to be very close to the minimum contrast already; when I changed the desired color, not much happens due to it being clamped. We may want to consider an alternative behavior if we want to respect the minimum contrast.

I also noticed that Chrome is not consistent with Windows conventions in a much more significant way. The window frame in Chrome is grey when active and a lighter grey when inactive. In other Windows applications, it is always white, and the shade doesn't change when the window goes inactive. I attached two images demonstrating this.

So if platform conventions are the concern, we might have to make larger changes.

Deleted: chrome_active_explorer_inactive.png 28.2 KB

	chrome_active_explorer_inactive.png 28.2 KB View Download

Deleted: chrome_inactive_explorer_active.png 37.6 KB

	chrome_inactive_explorer_active.png 37.6 KB View Download

The switch to use two different colors to indicate active/inactive frames was made recently.  If we make the changes on this bug for Win 10, I think we should revert the light grey inactive frame on Win 10 as well.  We shouldn't go from grey to white for the active frame, though, because that would require us to change the toolbar color or else have a stroke in the default case, and neither is palatable.

Regarding your first paragraph, you may need to reduce the minimum contrast or make other changes.  The problem space is complex; custom themes and custom system titlebar colors both play in.

So you're suggesting having both the active and inactive frame be the same, darker grey? That might fix the contrast issue too, at least in the default theme case.

For now, I'm just going to submit a change to add COLOR_TAB_TEXT_INACTIVE to themes. This seems appropriate, since there is already COLOR_BACKGROUND_TAB_TEXT_INACTIVE.

Yeah, I think we should use the darker grey for both active and inactive windows when we're indicating activity with this method, unless the user has custom active/inactive window titlebar colors set.

We don't let theme authors specify the inactive window toolbar color and I don't think we should let them specify the inactive window active tab color either.  I think we should just compute this dynamically in the nonclient frame view.

It's just the tab text color, no? BrowserNonClientFrameView::GetTabBackgroundColor always gets the active tab color from COLOR_TOOLBAR. BrowserNonClientFrameView::GetTabForegroundColor (which gets the text color, I believe) currently fetches COLOR_TAB_TEXT if the tab is active, and otherwise fetches one of COLOR_BACKGROUND_TAB_TEXT or COLOR_BACKGROUND_TAB_TEXT_INACTIVE depending on whether the window is active.

This naming could be cleaned up. Unless I'm misunderstanding it, there's two separate meanings of both background/foreground and active/inactive:

* BACKGROUND_TAB colors are used if the tab is in the background, i.e. the tab is inactive
* GetTabBackgroundColor() gets the color of the background of the tab, whether the tab is active or inactive
* _INACTIVE in the color name (e.g. COLOR_BACKGROUND_TAB_INACTIVE) refers to whether the window is active.

I didn't type that out very clearly. For clarification, here's the different meanings I see:

* Background/foreground can refer to the color of the tab vs the color of the tab text, or to whether the tab is the active tab or not.
* Active/inactive can refer to whether the window is active, or whether the tab is active.

These different meanings are clashing here.

You got it all correct.  My argument was, I don't think we should have a customizable COLOR_TAB_TEXT_INACTIVE.  If we have a non-customizable internal one because it makes your life easier implementationwise, so be it.

Oh, I guess I misunderstood. Yeah, I think having an internal property would make the implementation simpler and more understandable.

A potential downside to that is that we have to plumb that constant into a variety of places, and it implies to readers that the color might differ in arbitrary ways from the active tab.  I was thinking instead of something like:

SkColor GlassBrowserFrameView::GetTabForegroundColor(...) override {
  SkColor color = BrowserNonClientFrameView::GetTabForegroundColor(...);
  if (is windows 10 && !ShouldPaintAsActive())
    color = AlphaBlend(color, GetTabBackgroundColor(...), 40%);
  return color;
}

That's an alternative, but what about other platforms where we might also want to change the foreground color? Would we just implement similar platform-specific code for each one? That could be the best choice if we expect to handle this differently on different platforms.

At the moment, I don't think we have plans to do anything other than on Win 10 specifically.

My only input here is that we should avoid doing anything that reduces contrast to below accessibility minima.

That's a hard line for me. I know Chrome isn't 100% consistent with accessibility standards but we should never be doing anything to move us *away* from accessibility.

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

commit 059b37110ad2d6dcd3c177b66f26bbfdfe197386
Author: Collin Baker <collinbaker@chromium.org>
Date: Mon Sep 17 23:31:27 2018

Add COLOR_TAB_TEXT_INACTIVE theme property

Allows the active tab's text color to be different when the window is inactive.

BUG=859243

Change-Id: Icf9ae2aee881a4927d70d78ee1c7773c016ad1bc
Reviewed-on: https://chromium-review.googlesource.com/1196010
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591874}
[modify] https://crrev.com/059b37110ad2d6dcd3c177b66f26bbfdfe197386/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/059b37110ad2d6dcd3c177b66f26bbfdfe197386/chrome/browser/themes/theme_properties.cc
[modify] https://crrev.com/059b37110ad2d6dcd3c177b66f26bbfdfe197386/chrome/browser/themes/theme_properties.h
[modify] https://crrev.com/059b37110ad2d6dcd3c177b66f26bbfdfe197386/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/059b37110ad2d6dcd3c177b66f26bbfdfe197386/chrome/browser/ui/views/frame/browser_non_client_frame_view_unittest.cc

Follow up from email: 

UX Preference (leave as-is)

1. Inactive window tab text: 100%
2. Inactive window frame: #E7EAED (no change)
3. Window controls: dimmed (no change)


Rationale: 

- Compliant with a combination of current WIN and Mac conventions
- tab text (active and inactive) remains readable
- Some resemblance of contrast between inactive and active with frame color 


Full notes: 
https://docs.google.com/document/d/1dYwhZsXpqGZwBFIufmyOFWFDuPgyGum5C3VgnGCuF68/edit?hl=en

@34: I feel a little frustrated by this doc, because the conclusions all contradict what standard Windows conventions and behavior are.  Based on the evidence in the doc I assume someone determined the Windows conventions by looking at Edge, which isn't a win32 app and doesn't behave like anything else on the system.

Can you redo your decision-making process using a correct frame of reference on what Windows system conventions are?

@35: Thanks for the feedback. Updated the doc. Please advise. 

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

commit 9e28801fa50b407400a396699b7618badd4e5d47
Author: Collin Baker <collinbaker@chromium.org>
Date: Fri Nov 09 00:40:36 2018

Update tab text colors

This updates tab text colors to both enhance contrast of text over tab
backgrounds as well as contrast between active and inactive windows.

Bug: 859243, 881916
Change-Id: I98257077fd6a9b7cf9521f845ea40218dcf11a70
Reviewed-on: https://chromium-review.googlesource.com/c/1297185
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606668}
[modify] https://crrev.com/9e28801fa50b407400a396699b7618badd4e5d47/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc

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

commit d71e41b95b9d40bc36eaf23815fc63c0ed3b49fa
Author: Collin Baker <collinbaker@chromium.org>
Date: Fri Nov 16 17:30:46 2018

Fix tab colors for custom themes

This change:
* removes COLOR_TAB_TEXT_INACTIVE and instead simply alpha-blends
  COLOR_TAB_TEXT
* removes disabled unit test that uses COLOR_TAB_TEXT_INACTIVE
* uses COLOR_BACKGROUND_TAB_TEXT_INACTIVE directly if a theme
  provides it

Bug:  901610 , 859243
Change-Id: I7db13a7719284ac3e27ec3bfc122d6381430df26
Reviewed-on: https://chromium-review.googlesource.com/c/1334419
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608831}
[modify] https://crrev.com/d71e41b95b9d40bc36eaf23815fc63c0ed3b49fa/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/d71e41b95b9d40bc36eaf23815fc63c0ed3b49fa/chrome/browser/themes/theme_properties.cc
[modify] https://crrev.com/d71e41b95b9d40bc36eaf23815fc63c0ed3b49fa/chrome/browser/themes/theme_properties.h
[modify] https://crrev.com/d71e41b95b9d40bc36eaf23815fc63c0ed3b49fa/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/d71e41b95b9d40bc36eaf23815fc63c0ed3b49fa/chrome/browser/ui/views/frame/browser_non_client_frame_view_unittest.cc

This bug seems to have morphed into the general "tabstrip contrast" bug.

Collin, I hope you don't mind if I take this -- I'm deep into an implementation that addresses a bunch of issues and does fairly major surgery, and this hasn't seen action in some time.

Yep, no worries!