From ca911396a133f69c9e45ddde58a5625e8dff300e Mon Sep 17 00:00:00 2001 From: tophf Date: Thu, 13 Apr 2017 11:03:45 +0300 Subject: [PATCH] Don't use chrome://favicon chrome://favicon doesn't indicate an icon is missing in any way, it simply shows a placeholder instead. It also doesn't extrapolate from sub-pages so `example.com` won't have a favicon even if `example.com/subpage` has one. --- _locales/en/messages.json | 2 +- manage.js | 67 ++++++++------------------------------- manifest.json | 3 -- 3 files changed, 14 insertions(+), 58 deletions(-) diff --git a/_locales/en/messages.json b/_locales/en/messages.json index dfee7509..cb690d9e 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -295,7 +295,7 @@ "description": "Label for the checkbox that toggles applies-to favicons in the new UI on manage page" }, "manageFaviconsHelp": { - "message": "Stylus asks for chrome://favicon permission to retrieve the icons from browser cache. For non-cached icons Stylus uses external service https://www.google.com/s2/favicons", + "message": "Stylus uses an external service https://www.google.com/s2/favicons", "description": "Label for the checkbox that toggles applies-to favicons in the new UI on manage page" }, "manageMaxTargets": { diff --git a/manage.js b/manage.js index 500b342f..ad50c44f 100644 --- a/manage.js +++ b/manage.js @@ -14,10 +14,7 @@ const newUI = { newUI.renderClass(); const TARGET_TYPES = ['domains', 'urls', 'urlPrefixes', 'regexps']; -const GET_FAVICON_URL = { - builtin: 'chrome://favicon/size/16@2x/', - external: 'https://www.google.com/s2/favicons?domain=', -}; +const GET_FAVICON_URL = 'https://www.google.com/s2/favicons?domain='; const OWN_ICON = chrome.runtime.getManifest().icons['16']; const handleEvent = {}; @@ -77,6 +74,10 @@ function initGlobalEvents() { checkbox.onchange = () => installed.classList.toggle(className, checkbox.checked); } + $$('[data-toggle-on-click]').forEach(el => { + el.onclick = () => $(el.dataset.toggleOnClick).classList.toggle('hidden'); + }); + enforceInputRange($('#manage.newUI.targets')); setupLivePrefs([ @@ -87,28 +88,8 @@ function initGlobalEvents() { 'manage.newUI.targets', ]); - $('#manage.newUI').onchange = switchUI; - $('#manage.newUI.targets').oninput = switchUI; - $('#manage.newUI.targets').onchange = switchUI; - $('#manage.newUI.favicons').onchange = function() { - if (!this.checked) { - switchUI(); - return; - } - if (this.disabled) { - return; - } - this.disabled = true; - onPermissionsGranted({origins: ['chrome://favicon/']}).then( - switchUI, - () => (this.checked = false) - ).then( - () => (this.disabled = false) - ); - }; - $$('[data-toggle-on-click]').forEach(el => { - el.onclick = () => $(el.dataset.toggleOnClick).classList.toggle('hidden'); - }); + $$('[id^="manage.newUI"]') + .forEach(el => (el.oninput = (el.onchange = switchUI))); switchUI({styleOnly: true}); } @@ -206,12 +187,12 @@ function createStyleElement({style, name}) { } else if (newUI.favicons) { let favicon = ''; if (type == 'domains') { - favicon = 'http://' + targetValue; + favicon = GET_FAVICON_URL + targetValue; } else if (targetValue.startsWith('chrome-extension:')) { favicon = OWN_ICON; } else if (type != 'regexps') { - favicon = targetValue.includes('://') && targetValue.match(/^.*?:\/\/[^/]+/); - favicon = favicon ? favicon[0] : ''; + favicon = targetValue.includes('://') && targetValue.match(/^.*?:\/\/([^/]+)/); + favicon = favicon ? GET_FAVICON_URL + favicon[1] : ''; } if (favicon) { element.appendChild(document.createElement('img')).dataset.src = favicon; @@ -354,13 +335,10 @@ Object.assign(handleEvent, { loadFavicons(container = installed) { for (const img of container.getElementsByTagName('img')) { - const src = img.dataset.src; - if (!src) { - continue; + if (img.dataset.src) { + img.src = img.dataset.src; + delete img.dataset.src; } - img.src = src == OWN_ICON ? src - : GET_FAVICON_URL.builtin + (src.includes('://') ? src : 'http://' + src); - delete img.dataset.src; } } }); @@ -708,22 +686,3 @@ function findNextElement(style) { } return elements[elements[a].styleNameLowerCase <= nameLLC ? a + 1 : a]; } - - -function onPermissionsGranted(permissions) { - return new Promise((resolve, reject) => { - chrome.permissions.contains(permissions, alreadyGranted => { - if (alreadyGranted) { - resolve(); - } else { - chrome.permissions.request(permissions, granted => { - if (granted) { - resolve(); - } else { - reject(); - } - }); - } - }); - }); -} diff --git a/manifest.json b/manifest.json index f8301da2..d8d14f28 100644 --- a/manifest.json +++ b/manifest.json @@ -17,9 +17,6 @@ "contextMenus", "storage" ], - "optional_permissions": [ - "chrome://favicon/" - ], "background": { "scripts": ["messaging.js", "storage.js", "prefs.js", "background.js", "update.js"] },