Open Bug 740457 Opened 12 years ago Updated 2 years ago

setAndFetchFaviconForPage should invoke nsIFaviconDataCallback even if the favicon is unavailable.

Categories

(Toolkit :: Places, defect, P3)

defect

Tracking

()

People

(Reporter: Paolo, Unassigned)

References

Details

Per bug 737133 comment 2.
What's the status of this bug?  I'm updating Xmarks to work with the async APIs, but I'm not getting a callback from this function, which needs to happen on success and failure in order for our algorithm to continue processing.
no ETA, though would still be good to fix this.
On success it should be invoked already.
Fwiw you should not need to wait for it, since all of the operations are serialized you can just proceed after invoking setAndFetchFaviconForPage
Summary: setAndFetchFaviconForPage and setAndLoadFaviconForPage should invoke nsIFaviconDataCallback even if the favicon is unavailable. → setAndFetchFaviconForPage should invoke nsIFaviconDataCallback even if the favicon is unavailable.
Problem is that we sync favicons along with the bookmarks, so if we can't get the favicon data at the time of the sync, we can't send it to the server.  At this time I'm proceeding w/o the favicon sync functionality, which will upset some users if we have to release it in this state.  Also, am I correct that it should be calling back into onComplete as getFaviconURLForPage() does, not onFaviconDataAvailable as documented?
(In reply to Anatoly from comment #3)
> Problem is that we sync favicons along with the bookmarks, so if we can't
> get the favicon data at the time of the sync, we can't send it to the
> server.

Why do you use the set method to get the favicon, instead of getFaviconURLForPage or getFaviconDataForPage? Those methods callback is always invoked.

>  Also,
> am I correct that it should be calling back into onComplete as
> getFaviconURLForPage() does, not onFaviconDataAvailable as documented?

yes, the documentation is wrong, I will fix it.

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/mozIAsyncFavicons.idl
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsIFaviconService.idl#92
Ahh that would have helped, as getFaviconDataForPage isn't documented at https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/mozIAsyncFavicons.  I will try that instead.
That works.  Thanks!
(In reply to Marco Bonardo [:mak] from comment #4)
> >  Also,
> > am I correct that it should be calling back into onComplete as
> > getFaviconURLForPage() does, not onFaviconDataAvailable as documented?
> 
> yes, the documentation is wrong, I will fix it.

updated the callback documentation.
Fwiw, notice this callback has the "function" decorator, so you can just pass a function instead of a nsIFaviconDataCallback object.
Bug 745301 implemented favicons in Thunderbird folderpane and tabs for feeds.  Lack of callback required a hacky setTimeout.  History is not enabled, and while it could be it's not really necessary for the bug's purpose.

A callback should always be invoked here.  Then, it would be possible to implement a nice Promises chain and create much async joy.
patches are welcome, at this time we don't have the resources to work on this :/
Blocks: 745301
Still no resources after five years, huh? Mozilla sure have it tought.
Priority: -- → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.