Issue metadata
Sign in to add a comment
|
Regression: White favicon icon is seen on tabstrip on applying ‘Minnesota wild’ theme.
Reported by
dmascare...@etouch.net,
Jul 5 2016
|
||||||||||||||||||||||
Issue descriptionChrome Version:54.0.2788.0 (Official Build)503e06926312c99e6fe02e0d1a41c19eb6a06c9d-refs/heads/master@{#403706} OS: Mac( 10.10.5 ,10.11.4) Test url:1. https://chrome.google.com/webstore/detail/minnesota-wild-theme/coeepanojaeligknpijlmajikpnbiegc?hl=en 2. https://chromium.googlesource.com/chromium/src/+log/53.0.2750.0..53.0.2753.0?pretty=fuller&n=100000 or chrome://version/ Pre-condition: Install theme using above url. What steps will reproduce the problem? 1. Launch chrome and navigate to test url 2. 2. Observe the favicon icon of tabstrip. Actual: White favicon icon is seen. Expected: Coloured favicon icon should be seen. This is regression issue,broken in ‘M 54’ and below is narrow bisect: https://chromium.googlesource.com/chromium/src/+log/d7aeee6ee5d9274b6a1dbbe5bb3f67a77631248a..e89b7f8b8ee5ef77e80a96ff2be3012c156a51d6?pretty=fuller&n=1000 Suspecting: r403581 Good build:53.0.2785.0 Bad build:54.0.2787.0 Note: Issue is not seen on Windows and Linux OS.
,
Jul 6 2016
not sure how this could be my change, which shouldn't have affected the osx tabstrip. Do you see problems with other themes?
,
Jul 6 2016
I'm not sure how the favicon is getting its color. spqchan@ - what do you think could cause this?
,
Jul 6 2016
This might fix it: https://codereview.chromium.org/2126043002/ I need to test it out to make sure though, will do when I get back to my machine. Estade@, do you mind if I take this bug?
,
Jul 6 2016
Never mind, looks like this issue is different than what I was trying to fix. The color of the favicon is set here: https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm?rcl=0&l=1515 It's possible that it's set before the title text color got updated. I'll have to investigate more though
,
Jul 6 2016
re #4: be my guest :)
,
Jul 7 2016
Adding RB Label as this is a recent Regression. Please feel free to remove if not required. Thank You.
,
Jul 7 2016
I want clarify something: What color should the favicon always be? The same color as the title?
,
Jul 7 2016
It looks like it should be the same color as the close "x" when the mouse is not over it.
,
Jul 7 2016
Gotcha, thanks!
,
Jul 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7d57b7e232022e031e8a0908215f9ad4eee61117 commit 7d57b7e232022e031e8a0908215f9ad4eee61117 Author: spqchan <spqchan@chromium.org> Date: Thu Jul 14 22:51:00 2016 [Material][Mac] Fix Default Favicon's Color Default favicon now has the same color as the close button. This CL also makes sure that the favicon's color gets updated when the theme changes. CloseButtonColor has been refactored. BUG= 621015 , 625821 Review-Url: https://codereview.chromium.org/2126043002 Cr-Commit-Position: refs/heads/master@{#405612} [modify] https://crrev.com/7d57b7e232022e031e8a0908215f9ad4eee61117/chrome/browser/ui/cocoa/hover_close_button.h [modify] https://crrev.com/7d57b7e232022e031e8a0908215f9ad4eee61117/chrome/browser/ui/cocoa/hover_close_button.mm [modify] https://crrev.com/7d57b7e232022e031e8a0908215f9ad4eee61117/chrome/browser/ui/cocoa/tabs/alert_indicator_button_cocoa.mm [modify] https://crrev.com/7d57b7e232022e031e8a0908215f9ad4eee61117/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm [modify] https://crrev.com/7d57b7e232022e031e8a0908215f9ad4eee61117/chrome/browser/ui/cocoa/tabs/tab_view.h [modify] https://crrev.com/7d57b7e232022e031e8a0908215f9ad4eee61117/chrome/browser/ui/cocoa/tabs/tab_view.mm
,
Jul 18 2016
tested this on Mac OS X 10.11.5 using Chrome Canary # 54.0.2799.0 and it is working as intended. attached screencast for reference and adding TE - Verified labels.
,
Jul 20 2016
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dmascare...@etouch.net
, Jul 5 2016