Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Cache favicons (to prevent fetching each time Brave is reloaded) #2697

Closed
luixxiul opened this issue Jul 24, 2016 · 11 comments
Closed

Cache favicons (to prevent fetching each time Brave is reloaded) #2697

luixxiul opened this issue Jul 24, 2016 · 11 comments
Labels
feature/bookmarks fixed-with-brave-core This issue will automatically resolved with the replacement of Muon with Brave Core. misc/cache misc/favicon privacy wontfix

Comments

@luixxiul
Copy link
Contributor

Describe the issue you encountered: If favicons of bookmarks become 404, the favicons are also gone, which makes it difficult to notice that the bookmark is there, until #2075 is fixed. Also if a user starts Brave offline, no favicon will be displayed.

Expected behavior: When the user visit the site, Brave should store favicons in the Base64 format and replace them with it, and it should check if the favicon needs updating when the user revisits the site.

  • Brave Version: 0.11.1
@luixxiul
Copy link
Contributor Author

Also "clear site data" option from menubar should remove the favicon cache too.

@bsclifton
Copy link
Member

Caching would also be good because there is a delay in the bookmarks toolbar... for example, when you open a folder that has links. The delay is might only be 200 milliseconds before the image shows but it's noticeable.

@bbondy
Copy link
Member

bbondy commented Jul 24, 2016

@bsclifton someone complained about that on twitter recently. cc @BrendanEich

@bsclifton
Copy link
Member

For content pages like about:bookmarks, images are cached. However, images that are in the browser do not seem to be caching... like favicons in the bookmarks toolbar and the ones from the back/forward nav buttons (ex: stuff you can see when you load the shift+F8 dev tools).

When discussing with @bridiver, it seems that these are not using a public context when fetching, so it doesn't get cached. @bridiver did you have any info to add?

@bsclifton
Copy link
Member

bsclifton commented Dec 2, 2016

+1 from #5840; Assigned milestone

@bridiver
Copy link
Collaborator

bridiver commented Dec 2, 2016

I can bring in the history component from chromium

@bsclifton
Copy link
Member

@bridiver would this help with content loaded in a private context? ex: our main UI?

@bbondy bbondy modified the milestones: 0.13.4, 0.13.2 Dec 15, 2016
@bbondy bbondy modified the milestones: 0.13.5, 0.13.6 Feb 15, 2017
@luixxiul luixxiul added this to the Backlog milestone Mar 16, 2017
@luixxiul
Copy link
Contributor Author

I would be banned due to potential DOS attack:

clipboard01

If it is not easy to cache favicons for now, why don't we limit the maximum number of retry to fetch them, or prevent the browser from retrying to do so if the page is reloaded automatically?

@luixxiul luixxiul removed this from the Backlog milestone May 28, 2017
@luixxiul luixxiul added this to the 0.18.x milestone Jun 4, 2017
@luixxiul
Copy link
Contributor Author

luixxiul commented Jun 4, 2017

Also: requesting favicons each time on reload can be a strong indicator of the browser. It might be stronger than fingerprinting. CC @diracdeltas

@diracdeltas
Copy link
Member

diracdeltas commented Jun 5, 2017

favicon cache seems useful. it should be cleared at least whenever the user clears history.

we should be careful not to persist favicons from private tabs to disk. this was an issue previously before we changed the favicon context to non-persistent.

@bsclifton
Copy link
Member

Closing as wontfix as this will be addressed with Brave Core (coming soon). This is the new Chromium based UI which replaces the Muon based UI we use currently

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature/bookmarks fixed-with-brave-core This issue will automatically resolved with the replacement of Muon with Brave Core. misc/cache misc/favicon privacy wontfix
Projects
None yet
Development

No branches or pull requests

8 participants