514671 - Port Bug 480826 (Firefox should honour browser.chrome.favicons setting when opening Edit->Preferences->Applications)

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Philip Chee]

+++ This bug was initially created as a clone of Bug #480826 +++

[quote]
User-Agent:       Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.9.0.6) Gecko/2009022823 Firefox/3.0.6
Build Identifier: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.9.0.6) Gecko/2009022823 Firefox/3.0.6

When opening Edit->Preferences->Applications and selecting an Action a list
of actions pops up. If this list contains URLs for websites, their favicons
are loaded even if browser.chrome.favicons is set to false.

Reproducible: Always

Steps to Reproduce:
1. rm -rf .mozilla
2. Set browser.chrome.favicons to false
3. Open Edit->Preferences->Applications
4. Select the "Web Feed" action and view favicons being loaded
Actual Results:  
The favicons are being loaded even if browser.chrome.favicons is set to false.

Expected Results:  
The favicons should not be loaded.

Here is the patch I use:

--- browser/components/preferences/applications.js.ORI  2008-10-24 07:37:29.000000000 +0200
+++ browser/components/preferences/applications.js      2009-03-01 11:54:25.000000000 +0100
@@ -1894,7 +1894,7 @@
     // they'll only visit URLs derived from that template (i.e. with %s
     // in the template replaced by the URL of the content being handled).
 
-    if (/^https?/.test(uri.scheme))
+    if (/^https?/.test(uri.scheme) && this._prefSvc.getBoolPref("browser.chrome.favicons"))
       return uri.prePath + "/favicon.ico";
 
     return "";
[/quote]

Comment 1

[Philip Chee]

Attachment: Patch v1.0 honour |browser.chrome.favicons|

More or less a direct port of a one line patch.

Comment 2

[[e-mail]]

Comment on attachment 398665 [details] [diff] [review]
Patch v1.0 honour |browser.chrome.favicons|

>   _getIconURLForWebApp: function(aWebAppURITemplate) {
>     var uri = ioSvc.newURI(aWebAppURITemplate, null, null);
> 
>     // Unfortunately we need to use favicon.ico here, but we don't know
>     // about any other possibility to retrieve an icon for the web app/site
>     // without loading a specific full URL and parsing it for a possible
>     // shortcut icon.
> 
>-    return /^https?/.test(uri.scheme) ? uri.resolve("/favicon.ico") : "";
>+    return /^https?/.test(uri.scheme) && prefSvc.getBoolPref("browser.chrome.favicons") ?
>+           uri.resolve("/favicon.ico") : "";
The pref check should be the first thing in the function...

Comment 3

[Ian Neal]

Comment on attachment 398665 [details] [diff] [review]
Patch v1.0 honour |browser.chrome.favicons|

You should probably do the pref check in the function that calls _getIconURLForWebApp (i.e. _getIconURLForHandlerApp)

Comment 4

[Philip Chee]

Attachment: Patch v1.2 Move the pref check to _getIconURLForHandlerApp() [Checkin: Comment 6]

> You should probably do the pref check in the function that calls
> _getIconURLForWebApp (i.e. _getIconURLForHandlerApp)

Moved.

Comment 5

[[e-mail]]

Comment on attachment 398830 [details] [diff] [review]
Patch v1.2 Move the pref check to _getIconURLForHandlerApp()
[Checkin: Comment 6]

Good point, I didn't spot that it got called twice.

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 398830 [details] [diff] [review]
Patch v1.2 Move the pref check to _getIconURLForHandlerApp()
[Checkin: Comment 6]


http://hg.mozilla.org/comm-central/rev/28990692bc66