Closed Bug 1005064 Opened 7 years ago Closed 7 years ago

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

Categories

(MailNews Core :: Feed Reader, defect)

defect
Not set
normal

Tracking

(thunderbird31 fixed)

RESOLVED FIXED
Thunderbird 32.0
Tracking Status
thunderbird31 --- fixed

People

(Reporter: aryx, Assigned: alta88)

Details

Attachments

(2 files, 1 obsolete file)

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).
Attached patch fav.patchSplinter Review
Assignee: nobody → alta88
Attachment #8417691 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8417691 [details] [diff] [review]
fav.patch

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

LGTM! r=mkmelin
Attachment #8417691 - Flags: review?(mkmelin+mozilla) → review+
OS: Windows 8.1 → All
Hardware: x86_64 → All
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/c711ef16d7ee
Status: NEW → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 32.0
Comment on attachment 8417691 [details] [diff] [review]
fav.patch

this followup fix should follow bug 745301.
Attachment #8417691 - Flags: approval-comm-aurora?
Attachment #8417691 - Flags: approval-comm-aurora? → approval-comm-aurora+
Attached patch fav2.patch (obsolete) — Splinter Review
followup patch.  favicons that gecko cannot render will make setAndFetchFaviconForPage() throw; wrap in try.

example: http://www.contemporist.com/favicon.ico
Attachment #8433845 - Flags: review?(mkmelin+mozilla)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
Attachment #8433845 - Flags: review?(mkmelin+mozilla) → review+
Attached patch fav2.patchSplinter Review
updated comment.
Attachment #8433845 - Attachment is obsolete: true
Attachment #8437054 - Flags: review+
Keywords: checkin-needed
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
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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.
Attachment #8437054 - Flags: approval-comm-aurora?
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.
Attachment #8437054 - Flags: approval-comm-beta+
Attachment #8437054 - Flags: approval-comm-aurora?
Attachment #8437054 - Flags: approval-comm-aurora+
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?
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.
You need to log in before you can comment on or make changes to this bug.