993371 - Instantly get and use favicon for feed after importing feeds from opml file or adding single feed manually, don't require restart

Status: RESOLVED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Thank you for adding the favicon feature to the folder tree, alta88.

In Daily 20140408 on Windows 8.1, importing of an OPML file holding a list of ~25 feeds shows only the the favicon of three feeds (and the default one for the other ones). After closing and starting Thunderbird again (online required), it seems to load the remaining ones it can get. A good test case for this is likely the feed list from http://planet.mozilla.org/

Also in that version, I subscribed to http://rss.golem.de/rss.php?feed=ATOM1.0 and to see the favicon in the folder tree, a close and relaunch of Thunderbird was required.

Comment 1

[alta88]

yes, adds/changes in both folder pane and subscribe dialog could be improved.  the problem is that a folder is added first, and the tree paints before the feed is added to the db.  in order to be performant, whether a folder has a feed/the feed url is cached.

the cached favicon value for trees should be nulled on a feed folder subscription change.

Comment 2

[alta88]

Attachment: favicon2.patch

Comment 3

[alta88]

this might need the patch in bug 946279 first.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 8404405 [details] [diff] [review]
favicon2.patch

Review of attachment 8404405 [details] [diff] [review]:
-----------------------------------------------------------------

Didn't try it, but it looks reasonable to me.

::: mailnews/extensions/newsblog/content/FeedUtils.jsm
@@ +299,5 @@
> + * @param  string aValue          - value
> + */
> +  setFolderPaneProperty: function(aFolder, aProperty, aValue) {
> +    let win = Services.wm.getMostRecentWindow("mail:3pane");
> +    if (!aFolder || !win || !("gFolderTreeView" in win))

"gFolderTreeView" in win should always exist I'd think

Comment 5

[alta88]

Attachment: favicon2.patch

rebase and tweak for checkin.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/a35073be7f83