1005064 - Errors in FeedUtils.jsm when fetching favicons: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [mozIAsyncFavicons.setAndFetchFaviconForPage] and dom.head is undefined

Status: RESOLVED FIXED

(MailNews Core :: Feed Reader, defect)

Description

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

Thunderbird Daily 20140502 on Windows 8.1

I get the following error messages while being online for the first time after the favicon support for feeds landed:

Error: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [mozIAsyncFavicons.setAndFetchFaviconForPage]
Source File: resource:///modules/FeedUtils.jsm
Line: 434

Error: dom.head is undefined
Source File: resource:///modules/FeedUtils.jsm
Line: 421

If the subscriptions are needed, please let me know (the urls for some of them aren't valid anymore).

Comment 1

[alta88]

Attachment: fav.patch

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 8417691 [details] [diff] [review]
fav.patch

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

LGTM! r=mkmelin

Comment 3

[Ryan VanderMeulen [:RyanVM]]

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

Comment 4

[alta88]

Comment on attachment 8417691 [details] [diff] [review]
fav.patch

this followup fix should follow bug 745301.

Comment 5

[alta88]

Attachment: fav2.patch

followup patch.  favicons that gecko cannot render will make setAndFetchFaviconForPage() throw; wrap in try.

example: http://www.contemporist.com/favicon.ico

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 8433845 [details] [diff] [review]
fav2.patch

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

r=mkmelin, but add the comment that it's in try because for the case we get favicons that gecko cannot render

Comment 7

[alta88]

Attachment: fav2.patch

updated comment.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

In the future, please file a new bug for follow-up work instead of attaching patches to a bug that was resolved a month ago.

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

Comment 9

[alta88]

Comment on attachment 8437054 [details] [diff] [review]
fav2.patch


this followup fix should also follow bug 745301; blank favicons in folderpane for bad urls result, risk is very low.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8437054 [details] [diff] [review]
fav2.patch

[Triage Comment]
Ok, we should take this to beta as well there, now we're post merges.

Comment 11

[alta88]

in the past i think you checked into branches at some point if these flags were set, but it doesn't seem that's happened yet.  is a checkin-needed or such necessary?

Comment 12

[Mark Banner (:standard8)]

These landed on beta, I must have forgotten to comment:

http://hg.mozilla.org/releases/comm-beta/rev/eea7a05a3085
http://hg.mozilla.org/releases/comm-beta/rev/4e00ecfd4430

They were taken across to aurora at the last merge.