Themes which don't set tints/images should not have visible background tabs |
||||||
Issue descriptionIn refresh, background tabs are meant to blend into the frame. Where themes don't intentionally do something else (by setting explicit tab images or tints), they should do likewise. This requires several parts: (1) Change the default background tab tint to (-1, -1, -1), i.e. no tint. (2) Add the concept of inactive window background tab images/colors, so tabs can continue to "look invisible" when the window is inactivated. (3) Add background tab text colors and compute them automatically. This also allows themes to override the default behavior if they don't want it.
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3b7859cbe2547755a07abf56251808959f51a91e commit 3b7859cbe2547755a07abf56251808959f51a91e Author: Peter Kasting <pkasting@chromium.org> Date: Fri Jul 27 21:35:39 2018 Add background tab text colors for inactive and incognito windows. With the default behavior of Chrome being to not show a tab background, we want to change themes to default to that as well, unless the theme author actually supplies an image/color for this explicitly. To do this, we need to support different tab text colors for the cases where the frame colors can vary. This adds the framework for that support. Right now, the default is to use the same text colors in inactive windows as active ones. This also gives themes that want normal and incognito windows to differ greatly (as the built-in theme's does) the ability to implement that and still have readable tab titles. Bug: 866672 Change-Id: I032a72bc799d0675c517db10283513a30db23328 Reviewed-on: https://chromium-review.googlesource.com/1152418 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Evan Stade <estade@chromium.org> Cr-Commit-Position: refs/heads/master@{#578807} [modify] https://crrev.com/3b7859cbe2547755a07abf56251808959f51a91e/chrome/browser/themes/browser_theme_pack.cc [modify] https://crrev.com/3b7859cbe2547755a07abf56251808959f51a91e/chrome/browser/themes/browser_theme_pack_unittest.cc [modify] https://crrev.com/3b7859cbe2547755a07abf56251808959f51a91e/chrome/browser/themes/increased_contrast_theme_supplier.cc [modify] https://crrev.com/3b7859cbe2547755a07abf56251808959f51a91e/chrome/browser/themes/theme_properties.cc [modify] https://crrev.com/3b7859cbe2547755a07abf56251808959f51a91e/chrome/browser/themes/theme_properties.h [modify] https://crrev.com/3b7859cbe2547755a07abf56251808959f51a91e/chrome/browser/themes/theme_service.cc [modify] https://crrev.com/3b7859cbe2547755a07abf56251808959f51a91e/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67f5bec382fee417e3b8a79888d4104e569720cb commit 67f5bec382fee417e3b8a79888d4104e569720cb Author: Scott Violet <sky@chromium.org> Date: Fri Jul 27 23:37:55 2018 Revert "Add background tab text colors for inactive and incognito windows." This reverts commit 3b7859cbe2547755a07abf56251808959f51a91e. Reason for revert: Reverting in hopes of get unit_tests passing on chromeos bot. This appears to be next in the chain to revert. Original change's description: > Add background tab text colors for inactive and incognito windows. > > With the default behavior of Chrome being to not show a tab background, we want > to change themes to default to that as well, unless the theme author actually > supplies an image/color for this explicitly. To do this, we need to support > different tab text colors for the cases where the frame colors can vary. This > adds the framework for that support. > > Right now, the default is to use the same text colors in inactive windows as > active ones. > > This also gives themes that want normal and incognito windows to differ greatly > (as the built-in theme's does) the ability to implement that and still have > readable tab titles. > > Bug: 866672 > Change-Id: I032a72bc799d0675c517db10283513a30db23328 > Reviewed-on: https://chromium-review.googlesource.com/1152418 > Commit-Queue: Peter Kasting <pkasting@chromium.org> > Reviewed-by: Evan Stade <estade@chromium.org> > Cr-Commit-Position: refs/heads/master@{#578807} TBR=pkasting@chromium.org,estade@chromium.org,bsep@chromium.org Change-Id: I5e3a5772cea3f7a0d3f790d27dcccc66c51cb161 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 866672 Reviewed-on: https://chromium-review.googlesource.com/1154067 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#578860} [modify] https://crrev.com/67f5bec382fee417e3b8a79888d4104e569720cb/chrome/browser/themes/browser_theme_pack.cc [modify] https://crrev.com/67f5bec382fee417e3b8a79888d4104e569720cb/chrome/browser/themes/browser_theme_pack_unittest.cc [modify] https://crrev.com/67f5bec382fee417e3b8a79888d4104e569720cb/chrome/browser/themes/increased_contrast_theme_supplier.cc [modify] https://crrev.com/67f5bec382fee417e3b8a79888d4104e569720cb/chrome/browser/themes/theme_properties.cc [modify] https://crrev.com/67f5bec382fee417e3b8a79888d4104e569720cb/chrome/browser/themes/theme_properties.h [modify] https://crrev.com/67f5bec382fee417e3b8a79888d4104e569720cb/chrome/browser/themes/theme_service.cc [modify] https://crrev.com/67f5bec382fee417e3b8a79888d4104e569720cb/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
,
Jul 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22 commit 4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22 Author: Peter Kasting <pkasting@chromium.org> Date: Sat Jul 28 17:36:33 2018 Reland "Add background tab text colors for inactive and incognito windows." This is a reland of 3b7859cbe2547755a07abf56251808959f51a91e Original change's description: > Add background tab text colors for inactive and incognito windows. > > With the default behavior of Chrome being to not show a tab background, we want > to change themes to default to that as well, unless the theme author actually > supplies an image/color for this explicitly. To do this, we need to support > different tab text colors for the cases where the frame colors can vary. This > adds the framework for that support. > > Right now, the default is to use the same text colors in inactive windows as > active ones. > > This also gives themes that want normal and incognito windows to differ greatly > (as the built-in theme's does) the ability to implement that and still have > readable tab titles. > > Bug: 866672 > Change-Id: I032a72bc799d0675c517db10283513a30db23328 > Reviewed-on: https://chromium-review.googlesource.com/1152418 > Commit-Queue: Peter Kasting <pkasting@chromium.org> > Reviewed-by: Evan Stade <estade@chromium.org> > Cr-Commit-Position: refs/heads/master@{#578807} Bug: 866672 Change-Id: I2d80ccb2db306655b666d16588bfae59f087639b TBR: estade, bsep Reviewed-on: https://chromium-review.googlesource.com/1154149 Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#578929} [modify] https://crrev.com/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22/chrome/browser/themes/browser_theme_pack.cc [modify] https://crrev.com/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22/chrome/browser/themes/browser_theme_pack_unittest.cc [modify] https://crrev.com/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22/chrome/browser/themes/increased_contrast_theme_supplier.cc [modify] https://crrev.com/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22/chrome/browser/themes/theme_properties.cc [modify] https://crrev.com/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22/chrome/browser/themes/theme_properties.h [modify] https://crrev.com/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22/chrome/browser/themes/theme_service.cc [modify] https://crrev.com/4fadc40c46e13d3d5ea2c8d6eed9f159888fdd22/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
,
Jul 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7afb4f391067e7f043fc79d27bda0a7b27d33495 commit 7afb4f391067e7f043fc79d27bda0a7b27d33495 Author: Peter Kasting <pkasting@chromium.org> Date: Sat Jul 28 20:18:54 2018 Tint background tabs like the frame by default in refresh. This means themes which do not set a background tab tint or image will get background tabs that blend into the frame, just like the default theme. Bug: 866672 Change-Id: I9ad1544f6fbc3b6bcc134312fab329d7f4554694 Reviewed-on: https://chromium-review.googlesource.com/1152522 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#578935} [modify] https://crrev.com/7afb4f391067e7f043fc79d27bda0a7b27d33495/chrome/browser/themes/theme_properties.cc [modify] https://crrev.com/7afb4f391067e7f043fc79d27bda0a7b27d33495/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
,
Jul 28
,
Jul 29
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the 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
,
Jul 29
Could you pls update which CL you're requesting a merge to M69 for? Also how safe is the CL to merge? Thank you.
,
Jul 30
Both CLs on this bug: comment 2 (the reland in comment 4 is identical) and comment 5. This stuff isn't as safe as I'd like, but our hands are tied: we need this theme work to make Refresh work well with custom themes in GM2. I'm comfortable merging it and addressing any issues that happen to appear before stable.
,
Jul 30
Thank you pkasting@. Approving merge to M69 branch 3497 for CLs listed at #9 based on comment #9. Please merge before 3:00 PM PT, tomorrow, Monday so we can pick it up for next week Dev/Beta release. Thank you.
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/192b6e7358f31877329cb923e002871d08c3fca5 commit 192b6e7358f31877329cb923e002871d08c3fca5 Author: Peter Kasting <pkasting@chromium.org> Date: Mon Jul 30 20:40:44 2018 Add background tab text colors for inactive and incognito windows. With the default behavior of Chrome being to not show a tab background, we want to change themes to default to that as well, unless the theme author actually supplies an image/color for this explicitly. To do this, we need to support different tab text colors for the cases where the frame colors can vary. This adds the framework for that support. Right now, the default is to use the same text colors in inactive windows as active ones. This also gives themes that want normal and incognito windows to differ greatly (as the built-in theme's does) the ability to implement that and still have readable tab titles. Bug: 866672 Change-Id: I032a72bc799d0675c517db10283513a30db23328 Reviewed-on: https://chromium-review.googlesource.com/1152418 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Evan Stade <estade@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#578807}(cherry picked from commit 3b7859cbe2547755a07abf56251808959f51a91e) Reviewed-on: https://chromium-review.googlesource.com/1155493 Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#242} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/192b6e7358f31877329cb923e002871d08c3fca5/chrome/browser/themes/browser_theme_pack.cc [modify] https://crrev.com/192b6e7358f31877329cb923e002871d08c3fca5/chrome/browser/themes/browser_theme_pack_unittest.cc [modify] https://crrev.com/192b6e7358f31877329cb923e002871d08c3fca5/chrome/browser/themes/increased_contrast_theme_supplier.cc [modify] https://crrev.com/192b6e7358f31877329cb923e002871d08c3fca5/chrome/browser/themes/theme_properties.cc [modify] https://crrev.com/192b6e7358f31877329cb923e002871d08c3fca5/chrome/browser/themes/theme_properties.h [modify] https://crrev.com/192b6e7358f31877329cb923e002871d08c3fca5/chrome/browser/themes/theme_service.cc [modify] https://crrev.com/192b6e7358f31877329cb923e002871d08c3fca5/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4d3acb03701847403f6326ad993a17118a03ea4a commit 4d3acb03701847403f6326ad993a17118a03ea4a Author: Peter Kasting <pkasting@chromium.org> Date: Mon Jul 30 20:41:49 2018 Tint background tabs like the frame by default in refresh. This means themes which do not set a background tab tint or image will get background tabs that blend into the frame, just like the default theme. Bug: 866672 Change-Id: I9ad1544f6fbc3b6bcc134312fab329d7f4554694 Reviewed-on: https://chromium-review.googlesource.com/1152522 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#578935}(cherry picked from commit 7afb4f391067e7f043fc79d27bda0a7b27d33495) Reviewed-on: https://chromium-review.googlesource.com/1155494 Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#244} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/4d3acb03701847403f6326ad993a17118a03ea4a/chrome/browser/themes/theme_properties.cc [modify] https://crrev.com/4d3acb03701847403f6326ad993a17118a03ea4a/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
,
Jul 30
,
Jul 30
Is CL listed at #12 need a merge to M69? If yes, pls request a merge once change is baked/verified in canary. Thank you.
,
Jul 30
Sorry CL listed at #12 is already in M69 branch. Pls ignore comment #14.
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f4837db49d1301b0d769a0b53c9942c6ab001502 commit f4837db49d1301b0d769a0b53c9942c6ab001502 Author: Peter Kasting <pkasting@chromium.org> Date: Mon Jul 30 21:27:46 2018 Bump theme pack version. This was supposed to have been in https://chromium-review.googlesource.com/c/chromium/src/+/1152522 . Bug: 866672 Change-Id: I4b2076e13b1dc0ae2db0eec51775fb80790b90d7 TBR: estade Reviewed-on: https://chromium-review.googlesource.com/1155486 Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#579176} [modify] https://crrev.com/f4837db49d1301b0d769a0b53c9942c6ab001502/chrome/browser/themes/browser_theme_pack.cc
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1210e1d994eba8ed00e125a88dde54799ff09b05 commit 1210e1d994eba8ed00e125a88dde54799ff09b05 Author: Peter Kasting <pkasting@chromium.org> Date: Mon Jul 30 21:31:44 2018 Bump theme pack version. This was supposed to have been in https://chromium-review.googlesource.com/c/chromium/src/+/1152522 . Bug: 866672 Change-Id: I4b2076e13b1dc0ae2db0eec51775fb80790b90d7 TBR: estade Reviewed-on: https://chromium-review.googlesource.com/1155486 Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#579176}(cherry picked from commit f4837db49d1301b0d769a0b53c9942c6ab001502) Reviewed-on: https://chromium-review.googlesource.com/1155612 Cr-Commit-Position: refs/branch-heads/3497@{#249} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/1210e1d994eba8ed00e125a88dde54799ff09b05/chrome/browser/themes/browser_theme_pack.cc
,
Aug 14
+abdulsyed@ fyi, M69 merges taken for Proj-MdRefresh . |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by robliao@chromium.org
, Jul 24