Last Comment Bug 856406 - Tabbed browser should use favicon sevice in a privacy-aware way
: Tabbed browser should use favicon sevice in a privacy-aware way
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: unspecified
: All All
: -- normal (vote)
: seamonkey2.20
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
Depends on:
Blocks: 460895
  Show dependency treegraph
 
Reported: 2013-03-31 03:05 PDT by neil@parkwaycc.co.uk
Modified: 2013-04-05 16:07 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
Possible patch (3.31 KB, patch)
2013-03-31 03:08 PDT, neil@parkwaycc.co.uk
philip.chee: review+
iann_bugzilla: approval‑comm‑aurora+
iann_bugzilla: approval‑comm‑beta+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2013-03-31 03:05:11 PDT
Not exactly sure what it achieves but it probably avoids storing cookies for instance.

Compare bug 856322.
Comment 1 neil@parkwaycc.co.uk 2013-03-31 03:08:40 PDT
Created attachment 731624 [details] [diff] [review]
Possible patch
Comment 2 Philip Chee 2013-04-01 03:28:40 PDT
Comment on attachment 731624 [details] [diff] [review]
Possible patch

> -      <field name="mMissedIconCache">
> -        null
Good catch!

>       <field name="mFaviconService" readonly="true">
>         Components.classes["@mozilla.org/browser/favicon-service;1"]
>                   .getService(Components.interfaces.nsIFaviconService)
>                   .QueryInterface(Components.interfaces.mozIAsyncFavicons);
>       </field>

While you're at it, get rid of the QI and getService(mozIAsyncFavicons) directly. I think mozIAsyncFavicons/nsIFaviconService have ClassInfo because when I inspect mozIAsyncFavicons, I get:

> [xpconnect wrapped (nsISupports, nsIFaviconService, mozIAsyncFavicons, nsITimerCallback)]

r=me from code inspection.
Comment 3 neil@parkwaycc.co.uk 2013-04-01 16:13:05 PDT
Pushed comm-central changeset 04bd0d8809c9.
Comment 4 neil@parkwaycc.co.uk 2013-04-02 13:54:09 PDT
Comment on attachment 731624 [details] [diff] [review]
Possible patch

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: Potential favicon information leak
Testing completed (on m-c, etc.): None
Risk to taking this patch (and alternatives if risky): 
String changes made by this patch: None
Comment 5 neil@parkwaycc.co.uk 2013-04-03 13:52:02 PDT
https://hg.mozilla.org/releases/comm-aurora/rev/75b4b6fe8b77
Comment 6 neil@parkwaycc.co.uk 2013-04-05 16:04:52 PDT
(In reply to Philip Chee from comment #2)
> (From update of attachment 731624 [details] [diff] [review])
> >       <field name="mFaviconService" readonly="true">
> >         Components.classes["@mozilla.org/browser/favicon-service;1"]
> >                   .getService(Components.interfaces.nsIFaviconService)
> >                   .QueryInterface(Components.interfaces.mozIAsyncFavicons);
> >       </field>
> 
> While you're at it, get rid of the QI and getService(mozIAsyncFavicons)
> directly. I think mozIAsyncFavicons/nsIFaviconService have ClassInfo because
> when I inspect mozIAsyncFavicons, I get:
> 
> > [xpconnect wrapped (nsISupports, nsIFaviconService, mozIAsyncFavicons, nsITimerCallback)]
> 
> r=me from code inspection.

Sorry, I forgot about this. (But I should get the constants from Components.interfaces.nsIFaviconService at the same time.)
Comment 7 neil@parkwaycc.co.uk 2013-04-05 16:07:16 PDT
http://hg.mozilla.org/releases/comm-beta/rev/73971e6fef8d

Note You need to log in before you can comment on or make changes to this bug.