Bug 728168 made two changes: 1. call setAndFetchFaviconForPage instead of setAndLoadFaviconForPage. AFAIU this somehow works despite the former being part of mozIAsyncFavicons. 2. dropped used of getFaviconLinkForIcon (seems using this there was wrong in the first place, see bug 728168 comment 0) See bug 728168 comment 2 for how to test this if need be (I didn't).
Attachment #620035 - Flags: review?(neil)
Comment on attachment 620035 [details] [diff] [review] patch >- this._faviconService.setAndLoadFaviconForPage(readerURI, faviconURI, false, >+ this._faviconService.setAndFetchFaviconForPage(readerURI, faviconURI, false, "somehow works"? Why not change the getter for _faviconService to use the correct interface? (Renaming the variable is optional.) >+ var notificationIcon = uri.prePath + "/favicon.ico"; Interesting, suite prefers to use uri.resolve("/favicon.ico") except ironically later in this very file!
Comment on attachment 620035 [details] [diff] [review] patch r- because it needs the right interface on the getter. Also bz had a very slight preference for .resolve("/favicon.ico") too.
Attachment #620035 - Flags: review?(neil) → review-
Comment on attachment 621458 [details] [diff] [review] patch v1a [Checkin: Comment 4] http://hg.mozilla.org/comm-central/rev/664d3445e151
Attachment #621458 - Attachment description: patch v1a → patch v1a [Checkin: Comment 4]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.12
You need to log in before you can comment on or make changes to this bug.