setAndFetchFaviconForPage should invoke nsIFaviconDataCallback even if the favicon is unavailable.

NEW
Unassigned

Status

()

Toolkit
Places
P3
normal
6 years ago
a year ago

People

(Reporter: Paolo, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
Per bug 737133 comment 2.

Comment 1

5 years ago
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.

Comment 3

5 years ago
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

Comment 5

5 years ago
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.

Comment 6

5 years ago
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.

Comment 8

4 years ago
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 :/

Updated

4 years ago
Blocks: 745301

Comment 10

2 years ago
Still no resources after five years, huh? Mozilla sure have it tought.
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.