368745 - Use nsIFaviconService in Feed Preview

Status: RESOLVED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect, P3)

Description

[Asaf Romano (gone)]

In places-builds, we should use nsIFaviconService in Feed Preview.

Comment 1

[Asaf Romano (gone)]

Attachment: v1

Comment 2

[Robert Sayre]

Comment on attachment 253429 [details] [diff] [review]
v1

>Index: browser/components/feeds/src/FeedWriter.js
...
>+    var faviconURL = null;
>+    try {
>+      faviconURL = faviconsSvc.getFaviconForPage(aURI);
>+    }
>+    catch(ex) { }
>+
>+    if (faviconURL) {

Why are we swallowing all exceptions? Shouldn't we at least check for a specific error that we expect?

Comment 3

[(not reading, please use [e-mail] instead)]

Comment on attachment 253429 [details] [diff] [review]
v1

r=sspitzer

as for robert's question, the favicon service will throw NS_ERROR_NOT_AVAILABLE when "the page is not found or it has no favicon".

see also bug #381396 about the places api using NS_ERROR_NOT_AVAILABLE in this manner.

Comment 4

[Asaf Romano (gone)]

Attachment: as checked in

Comment 5

[Asaf Romano (gone)]

mozilla/browser/components/feeds/src/FeedWriter.js 1.40