Taco Bell favicon flashes on mouseover

[petemill]

Description

Introduced with #12565. Haven't been able to reproduce on any other sites apart from tacobell.com.
Thanks for the find @bsclifton

Steps to Reproduce

1. Have a tab with tacobell.com's favicon
2. Mouse over tab

Actual result:

3. Icon flashes on and off

Expected result:
Icon remains visible

Reproduces how often:

Brave Version

about:brave info:

Reproducible on current live release:

No

Additional Information

[kjozwiak]

Also noticed this happening on https://badssl.com/ when QAing 0.22.13.. Example:

[jayyyin]

if it's ok I'd like to look into this...

[petemill]

@jayyyin go for it - please! Would be great to try and reproduce on something like codepen.io in order to experiment with fixing in a small reproducible scenario. It's possible that the gradient is interfering with certain types of favicon image encoding, as you've found in #13746. Forcing the favicon to paint on a new layer may help, but I think I tried that in initial investigation and promptly gave up for the time being!

[jayyyin]

@petemill yah, I found basically disabling the gradient fixes the issue, but I'm pretty sure that's not the way to go, how would I go about trying to paint it on a separate layer?

[jayyyin]

note the Favicon also "flashes" when you initially move the mouse into the tab area

[jayyyin]

This issue, seems to be related https://bugs.chromium.org/p/chromium/issues/detail?id=387146

[petemill]

@jayyyin actually I don't think that's related. This looks to me a css display / painting issue. The chromium issue you linked to is literally Chromium changing the favicon when the Url changes via pushState.

[jayyyin]

@petemill yah, I realized that after further investigation, I found another lead, the icon isn't cached correctly and is being redownloaded constantly, you can see using the networks tab that it is for some reason, any idea where the icon is being loaded?

[jayyyin]

I found that the cache-control for the headers of those files contain the 'no-cache' tag, I'm currently looking into how this could be handled, but this issue was in chromium at some point it seems but I can't seem to find how it was handled code wise, I'm having some trouble navigating the structure of the code to find where the code to download the favicons are, some help would be appreciated

[jayyyin]

@petemill, ok so from what I've learned making a new layer should fix the issue, since the icon shouldn't be getting invalidated anymore along with the CSS change for the gradient, so thus the no-cache issue won't affect it, but this should be noted for future reasons.

[jayyyin]

@petemill @kjozwiak TO sum up what I've found so far heres a blog post http://jyopensource.blogspot.ca/2018/04/brave-laptop-dev-interesting-issue-part.html

[petemill]

Good summary and find @JayYin. In that case, we should modify the fav icon react component to at least cache in memory the image, to prevent re-download on re-draw. One way would be to download the image and convert to data URI which is then passed to the css.

…

On Fri, Apr 20, 2018 at 2:08 PM jayyyin ***@***.***> wrote: TO sum up what I've found so far heres a blog post http://jyopensource.blogspot.ca/2018/04/brave-laptop-dev-interesting-issue-part.html — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13487 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAtRzNMPpPrbkPB18Dny0eb-dUe-k-e4ks5tqk5igaJpZM4SumeU> .