fix some favicon leaks #14654
fix some favicon leaks #14654
Conversation
Test Plan: 1. open browser console and inspect the urlbar icon 2. hit any search shortcut, ex: ':g' for google 3. notice that the favicon for the search engine is loaded as a local resource instead of as a 'http(s)://' URL
| * Returns path of a favicon resource | ||
| */ | ||
| const getPath = (name, ext = 'ico') => { | ||
| return path.join(__dirname, '..', '..', 'img', 'favicons', `${name}.${ext}`) |
There was a problem hiding this comment.
verified this works on packaged builds as well as in dev
There was a problem hiding this comment.
@diracdeltas does it not work just to use the same localImage property which calls getFaviconUrl?
There was a problem hiding this comment.
@petemill it doesn't work because you can't load chrome-extension:// URLs in a cross-origin context unless they're whitelisted in web_accessible_resources, but we'd rather not whitelist them for privacy reasons
| @@ -179,7 +179,7 @@ class ContextMenuItem extends ImmutableComponent { | |||
| width: iconSize | |||
| } | |||
|
|
|||
| const icon = this.props.contextMenuItem.get('icon') | |||
| const icon = !this.props.isTor && this.props.contextMenuItem.get('icon') | |||
There was a problem hiding this comment.
small nitpick here that IMO it would be preferrable to have the prop be more semantic, e.g. shouldDisplayIcon
| * Returns path of a favicon resource | ||
| */ | ||
| const getPath = (name, ext = 'ico') => { | ||
| return path.join(__dirname, '..', '..', 'img', 'favicons', `${name}.${ext}`) |
There was a problem hiding this comment.
@diracdeltas does it not work just to use the same localImage property which calls getFaviconUrl?
fix #14653
fix #14641
test plan: