New issue
Advanced search Search tips

Issue 869395 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

[Md-Refresh]: Font color is not applied to inactive tabs in Incognito mode

Project Member Reported by chelamcherla@chromium.org, Jul 31

Issue description

Chrome Version: 70.0.3508.0
OS: Windows, Linux , Mac

What steps will reproduce the problem?
(1) Launch chrome and install any theme (say https://chrome.google.com/webstore/detail/black-red-shards/jpjlkkaalgfbbegfnjoclhfidancjpch)
(2) Open 2-3 tabs in parent window, observe font color of inactive tabs
(3) Open 2-3 tabs in Incognito window and observe inactive tab font color

Expected: All the tabs should have added theme color as in normal mode.
Actual: Inactive and active tabs have different font color in incognito window.

Attaching screenshot for reference.

NOTE:  Issue 859055  is for Normal window and is fixed.

@kylixrd: Please look into this issue. Help in re-assigning to appropriate owner.

Thanks!
 
Labels: -FoundIn-70 -Target-70 -M-70 -Inhouse-HYD-Reported -TE-FeatureTesting M-69 OS-Chrome
Owner: pkasting@chromium.org
Status: Started (was: Assigned)
This was intentional, but on reflection, I think I was wrong.  We should copy the background tab text color to incognito windows by default to maintain previous behavior and let theme authors correct us where that's wrong.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 1

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

commit 3e5002e927985502e6cef30b6cf9c6f3084a7044
Author: Peter Kasting <pkasting@chromium.org>
Date: Wed Aug 01 17:18:13 2018

Copy the incognito background tab text color from the regular tab text color.

This preserves the "theme both normal and incognito windows" behavior of
COLOR_BACKGROUND_TAB_TEXT, which was the only color available until I added
incognito and inactive variants last week.  It makes some themes look more like
their authors intended (e.g. Black Red Shards).  OTOH, it makes some themes
unreadable because the copied text color doesn't work in the current treatment
of incognito tabs.  I'm not sure which route is better, but this route seems to
at least err on the side of author intent, so...

Bug:  869395 
Test: Install "Black Red Shards" theme, open an incognito window with multiple tabs, check that text is red on all tabs.
Change-Id: Idef3b07edd2b49b1acc440edaf8c31838350dd98
Reviewed-on: https://chromium-review.googlesource.com/1157867
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579845}
[modify] https://crrev.com/3e5002e927985502e6cef30b6cf9c6f3084a7044/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/3e5002e927985502e6cef30b6cf9c6f3084a7044/chrome/browser/themes/browser_theme_pack_unittest.cc

Labels: Merge-Request-69
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 2

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M69 branch 3497 before 3:00 PM PT today if possible. Thank you.
Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 3

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e82729637cae6ebb52b4c06693048fead09efa5

commit 6e82729637cae6ebb52b4c06693048fead09efa5
Author: Peter Kasting <pkasting@chromium.org>
Date: Fri Aug 03 17:32:17 2018

Copy the incognito background tab text color from the regular tab text color.

This preserves the "theme both normal and incognito windows" behavior of
COLOR_BACKGROUND_TAB_TEXT, which was the only color available until I added
incognito and inactive variants last week.  It makes some themes look more like
their authors intended (e.g. Black Red Shards).  OTOH, it makes some themes
unreadable because the copied text color doesn't work in the current treatment
of incognito tabs.  I'm not sure which route is better, but this route seems to
at least err on the side of author intent, so...

Bug:  869395 
Test: Install "Black Red Shards" theme, open an incognito window with multiple tabs, check that text is red on all tabs.
Change-Id: Idef3b07edd2b49b1acc440edaf8c31838350dd98
Reviewed-on: https://chromium-review.googlesource.com/1157867
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579845}(cherry picked from commit 3e5002e927985502e6cef30b6cf9c6f3084a7044)
Reviewed-on: https://chromium-review.googlesource.com/1162461
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#382}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/6e82729637cae6ebb52b4c06693048fead09efa5/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/6e82729637cae6ebb52b4c06693048fead09efa5/chrome/browser/themes/browser_theme_pack_unittest.cc

Sign in to add a comment