apps/communications/facebook/js/fb_sync_init.js: - sending notification on facebook sync errors - no payload

I don't understand how this is supposed to get triggered. I've forced a call to showNotification() in syncSuccess(), imported some facebook contacts with success, but I still see no notification being triggered.

The attached patch is a first change that enables the use of notification. This lacks of code to clear the notificaiton, though.

Alexandre, Jose could help you thought. I worked on FB integration long time ago Thx

:gerard-majax, Please ask me for a review (jmcf@tid.es) ;) thanks!

So basically the previous patch from gerard-majax works good, I'm just putting it on github after testing it (thanks Jose for the tips). There's no onClick behaviour, because there wasn't one specified before, but maybe we can think about something. Right now the notification gets flagged as read when clicked, but nothing else happens. Another approach I'm thinking about is to change directly the NotificationHelper.js so other apps takes advantage of the new API, but the change would affect more applications, so it would be more risky. Better change the apps one by one and get rid of the helper? Change it on another bug? Thoughts about that?

Comment on attachment 8434307 [details] Link to PR - https://github.com/mozilla-b2g/gaia/pull/20027 thanks Fernando

(In reply to Fernando Campo (:fcampo) from comment #5) > Created attachment 8434307 [details] > Link to PR - https://github.com/mozilla-b2g/gaia/pull/20027 > > So basically the previous patch from gerard-majax works good, I'm just > putting it on github after testing it (thanks Jose for the tips). > > There's no onClick behaviour, because there wasn't one specified before, but > maybe we can think about something. Right now the notification gets flagged > as read when clicked, but nothing else happens. > > Another approach I'm thinking about is to change directly the > NotificationHelper.js so other apps takes advantage of the new API, but the > change would affect more applications, so it would be more risky. Better > change the apps one by one and get rid of the helper? Change it on another > bug? Thoughts about that? Etienne did the NotificationHelper but I don't know if we still need that library ...

I was waiting for the ni to Etienne, but I thinks it's better to merge this (master - ab8cfa6ff52cbb86b7980bf96e6ee6bcb8a3fc70), and in case we need to change/delete the helper, we open a specific bug for it.

(In reply to Fernando Campo (:fcampo) from comment #8) > I was waiting for the ni to Etienne, but I thinks it's better to merge this > (master - ab8cfa6ff52cbb86b7980bf96e6ee6bcb8a3fc70), and in case we need to > change/delete the helper, we open a specific bug for it. This discussion happened a while ago in the blocking bug: we concluded there was too much complexity and risks for few gains in keeping the NotificationHelper.

Relaying what was said IRL, Alexandre is the best person to know if the NotificationHelper is still needed to work around a gecko bug. Looks like it isn't :)

Already replied.