Closed Bug 670341 Opened 9 years ago Closed 9 years ago

about:permissions queries hosts for favicons


(Firefox :: Preferences, defect)

Not set



Firefox 8


(Reporter: kdevel, Assigned: Margaret)




(1 file, 1 obsolete file)

User Agent:  
Build ID: 20110706164304

Steps to reproduce:

0. Use a profile which is use, not a fresh one.
1. Open new tab with web console (shift-ctrl-k)
2. about:permissions

Actual results:

Network access occurs. Fx tries to fetch some favicons

Expected results:

No network access occurs. Favicons shall be gathered only when a host is intentionally visited.
It is certainly not "nice" if favicons are grabbed when the Permission Manager is entered (Bug 658615).
Ever confirmed: true
OS: Other → All
Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20100101 Firefox/6.0
Mozilla/5.0 (X11; Linux x86_64; rv:7.0a2) Gecko/20110709 Firefox/7.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:8.0a1) Gecko/20110709 Firefox/8.0a1
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 658615
Actually, this is asking for the *opposite* of Bug 658615.
Component: General → Preferences
QA Contact: general → preferences
Resolution: DUPLICATE → ---
Blocks: 657961
This seems to be a problem with the favicon service API. What we really want is an API method that just returns locally stored favicon data if we have it, and does not try to connect to the host otherwise. It looks like this method exists in nsIFaviconService, but not mozIAsyncFavicons, so perhaps we need to add a new async method to do this?

I'm cc'ing Marco, since he knows more about the favicon service.
Blocks: 675945
can you just use a moz-anno:favicon:pageurl src for the icon?
Attached patch patch (obsolete) — Splinter Review
(In reply to comment #6)
> can you just use a moz-anno:favicon:pageurl src for the icon?

I wasn't aware that this was a way to get what I wanted. With this patch I'm not seeing any more network activity, but I'm still seeing favicons, so it solves the problem!
Assignee: nobody → margaret.leibovic
Attachment #550154 - Flags: review?(mak77)
Comment on attachment 550154 [details] [diff] [review]

Review of attachment 550154 [details] [diff] [review]:

per IRC discussion, this is fine, but adds 2 queries.  You should fire the https request before the http one, and ignore an eventual second callback if both exist.

::: browser/components/preferences/aboutPermissions.js
@@ +94,5 @@
>    getFavicon: function Site_getFavicon(aCallback) {
>      function faviconDataCallback(aURI, aDataLen, aData, aMimeType) {
>        try {
> +        // Avoid making a network connection to fetch the favicon
> +        aCallback("moz-anno:favicon:" + aURI.spec);

you can use gFaviconService.getFaviconLinkForIcon(aURI) to get automatically a moz-anno:favicon nsIURI (it just does the string concat practically)

@@ +102,5 @@
>      }
>      // Try to find favicion for both URIs. Callback will only be called if a
>      // favicon URI is found, so this means we'll always prefer the https favicon.
>      gFaviconService.getFaviconURLForPage(this.httpURI, faviconDataCallback);

gFaviconService should also be QI to Ci.mozIAsyncFavicons (you can .getService().QueryInterface() to get both)
Attachment #550154 - Flags: review?(mak77) → feedback+
nevermind QI, it's not needed, gavin pointed out we implement classinfo there, I honestly didn't recall :(
Attached patch patch v2Splinter Review
Attachment #550154 - Attachment is obsolete: true
Attachment #550186 - Flags: review?(mak77)
Comment on attachment 550186 [details] [diff] [review]
patch v2

Review of attachment 550186 [details] [diff] [review]:

::: browser/components/preferences/aboutPermissions.js
@@ +102,2 @@
>        try {
> +        // Get favicon data instead of fetching the favicon over the network.

nit: this comment looks a bit ambiguous, may get some love.
Attachment #550186 - Flags: review?(mak77) → review+
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 8
verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0) Gecko/20100101 Firefox/8.0
You need to log in before you can comment on or make changes to this bug.