Integrate Firefox notifications with notify-osd bling

Bug #501393 reported by Paul Sladen
120
This bug affects 21 people
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Medium
firefox (Ubuntu)
Fix Released
Wishlist
Unassigned
notify-osd (Ubuntu)
Won't Fix
Wishlist
Unassigned

Bug Description

Binary package hint: notify-osd

Firefox currently provides its own notifications, such as "All downloads have finished". Although similar in style to notify-ods' messages popup-box they are completely separate and unique to Firefox.

Ideally this could be integrated to use notify-ods.

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

You should document somewhere what the difference is between "@mozilla.org/system-alerts-service;1" and "@mozilla.org/alerts-service;1" (in particular, why we need both).

+ if (imgStatus == imgIRequest::STATUS_ERROR) {
+ // We have an error getting the image. Display the notification with no icon.
+ ShowAlert(NULL);
+ }

Should check mLoadedFrame here; if an error occurs after the first frame we could show the alert twice!

Did you copy the code for getting the application name from somewhere else? Can we share it somehow?

+ nsCAutoString mAlertTitle;
+ nsCAutoString mAlertText;

nsCString

+ PRBool mLoadedFrame;

PRPackedBool

+ nsCOMPtr<nsAlertsIconListener> alertListener = new nsAlertsIconListener();
+ return alertListener->InitAlertAsync(aImageUrl, aAlertTitle, aAlertText);

OOM check

Needs build-system review from Ted

Revision history for this message
In , Reed Loden (reed) wrote :

(From update of attachment 353279)
> { "GnomeVFS Service",
> NS_GNOMEVFSSERVICE_CID,
> NS_GNOMEVFSSERVICE_CONTRACTID,
>- nsGnomeVFSServiceConstructor }
>+ nsGnomeVFSServiceConstructor },
>+#ifdef MOZ_ENABLE_LIBNOTIFY
>+ { "Gnome Alerts Service",
>+ NS_SYSTEMALERTSSERVICE_CID,
>+ NS_SYSTEMALERTSERVICE_CONTRACTID,
>+ nsAlertsServiceConstructor },
>+#endif
> };

What if MOZ_ENABLE_LIBNOTIFY isn't #define'd... that trailing comma will be there. Will that cause a compilation error?

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

No. Notice how a trailing comma is on the end even if it is defined. Its all over the code to make it easier to add elements.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Created an attachment (id=353295)
Patch 2

Fix roc's comments.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

(From update of attachment 353295)
Ted, I need your approval on the build system changes.

Revision history for this message
In , L10n-mozilla (l10n-mozilla) wrote :

... make sure that --disable-compile-environment ignores the libnotify test. I'd do so even if it is currently not switched on by default.

Revision history for this message
In , Ted Mielczarek (ted-mielczarek) wrote :

(From update of attachment 353295)
Looks fine. As Pike said, that whole block in configure should really be inside
if test -z "$SKIP_LIBRARY_CHECKS";
fi

In fact, the dbus stuff up above (and maybe some more) should go inside a block like that as well.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Created an attachment (id=355343)
Patch 3

This adds support for that compile time check.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Just a note, this patch will be off by default until the build machines get libnotify dev headers installed (which I will talk to IT about after landing).

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

Pushed 9891b174d871

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

why did this end up in toolkit/system/gnome and not toolkit/components/alerts/src/gnome (similar to how the mac implementation is)?

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

So it can live in the dynamically linked libmozgnome, so we can link to libnotify and if it's not there the browser will still work.

Revision history for this message
In , Alexander Sack (asac) wrote :

Thanks! Would be nice if we'd get this on 1.9.1

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Yes it would be, but since string freeze has passed (or if not, coming up extremely soon) I don't know if it will make it considering quite a few strings may have to be changed to remove the implied "click me for further details" action.

I just hope 1.9.2 will be quicker than this "quick" 1.9.1 release cycle was :)

Revision history for this message
In , Alexander Sack (asac) wrote :

1.9.0 backport would require the backport in bug 393936 -> depends.

Revision history for this message
In , Alexander Sack (asac) wrote :

(From update of attachment 355343)
improvements on the linux desktop integration front seems to justify to request for 1.9.1 approval here. Even more so as ubuntu is getting a new notification system: http://www.markshuttleworth.com/archives/253

Thanks for considering this.

Revision history for this message
In , Mike Connor (mconnor) wrote :

(From update of attachment 355343)
Clearing the nomination to take this in 1.9.1, the lack of callback support feels like a usability regression, and I don't think we want to do that now, and probably ever. Need to sync with the right people to find a way forward.

Revision history for this message
In , Mike Connor (mconnor) wrote :

So, this never had ui-review, and it's clearly making user experience changes. I'm inclined to back it out until it's not a UX regression, or until we've decided that the regression is worth the integration win.

I'm completely unconvinced right now that the current behaviour is an improvement. I've read https://wiki.ubuntu.com/NotificationDesignGuidelines (now that Ubuntu made the spec public) and I've discussed it with the designers. I think they're wrong, and I don't think we win by following platform specs if the platform specs are less useful.

Revision history for this message
In , Dmose (dmose) wrote :

I have no particular opinion about this, but I've CCed a few folks who might.

Revision history for this message
In , Mike Connor (mconnor) wrote :

Actually, the API supports callbacks, but for some reason they weren't implemented? Bah, this is just multiple layers of wrong.

Revision history for this message
In , Mike Connor (mconnor) wrote :

So, it's possible that some notification servers won't support actions. It's silly, but it's possible. We should implement actions in toolkit, or back this out (as much as the ugly toaster is something I want to kill).

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

(In reply to comment #18)
> I'm completely unconvinced right now that the current behaviour is an
> improvement. I've read https://wiki.ubuntu.com/NotificationDesignGuidelines
> (now that Ubuntu made the spec public) and I've discussed it with the
> designers. I think they're wrong, and I don't think we win by following
> platform specs if the platform specs are less useful.

OK, but don't they get some leeway to be wrong? Not unlimited leeway, but some?

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

Also, is this a toolkit issue or a product issue? We could easily enable/disable native notifications per-product.

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

(In reply to comment #23)
> Also, is this a toolkit issue or a product issue? We could easily
> enable/disable native notifications per-product.
IMO this is a toolkit issue. This patch changes the behavior of how something works consistently on all platforms to something that isn't consistent (namely, callback support). A toolkit peer probably would have flagged that, but one didn't even look at this patch...

Revision history for this message
In , Mike Connor (mconnor) wrote :

I think this is a toolkit issue. We shouldn't partially implement libnotify and have nsIAlertsService be less capable on Linux. If distros disable actions in their notification server, that's their call, but I don't think our impl should ignore callbacks.

Revision history for this message
In , Mike Connor (mconnor) wrote :

As a note, even Ubuntu's implementation supports actions, it's just not "as shiny" right now. We should let the caller decide whether they want a callback or not.

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

OK. I believe at the time this was implemented Ubuntu was planning to not support callbacks. I'm glad this has changed.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

I can work on adding callback support, libnotify seems to have a good API for this at first glance.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Created an attachment (id=373102)
Add callbacks, and turn it on

This adds support for callbacks, and seems to work well.

I can still request ui-r if necessary (since I want to flip the switch now for 1.9.2) but with the callback support implemented as is in this patch, it works just like the toaster; clicking anywhere in the bubble (except the X) activates the callback, and the bubble disappears after the system default delay.

I'm still on ubuntu intrepid, I don't know what's happening for the final jaunty in the area of notifications (although jaunty will be well obsolete by the time 1.9.2 is even close to release...)

Revision history for this message
In , David-humphrey (david-humphrey) wrote :

> This adds support for callbacks, and seems to work well.

If we do take this, and get proper callback support, I'm wondering if we'd still consider it for 1.9.1, since it would let me switch the Thunderbird notifications over to use it.

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

Can we get some tests too please? Pretty please, with sugar, and spice, and everything nice?

Revision history for this message
In , Mike Connor (mconnor) wrote :

I think it's almost certainly too late for 1.9.1. A set of tests would be great in the service of changing that. There isn't a matter of "switching anything over" since it's the same API, it'll just be cleaner/nicer/better integrated.

As for ui-review, if it's now identical for the interaction model, and isn't requiring string changes because the behaviour is different, then ui-r=me.

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

+ nsAutoString alertCookie;

nsString (you normally don't want to use nsAuto* in structs)

+ nsAutoString mAlertCookie;

Same here

Instead of creating NotifyCallbackInfo, why don't you make the notify_ callbacks take the nsAlertsIconListener as their closure-data?

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

(In reply to comment #31)
> Can we get some tests too please? Pretty please, with sugar, and spice, and
> everything nice?

Litmus is the only way I can think of testing this, as this calls into the native toolkit and any tests I can think of would be no different to what we have for the toaster.

(In reply to comment #33)
> Instead of creating NotifyCallbackInfo, why don't you make the notify_
> callbacks take the nsAlertsIconListener as their closure-data?

I guess so, I was worried the nsAlertsIconListener would be destroyed before the callback happened but I suppose that's what XPCOM is for...

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

(In reply to comment #34)
> Litmus is the only way I can think of testing this, as this calls into the
> native toolkit and any tests I can think of would be no different to what we
> have for the toaster.
I don't even think we have those...

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

> I guess so, I was worried the nsAlertsIconListener would be destroyed before
> the callback happened but I suppose that's what XPCOM is for...

Right, NS_ADDREF the nsAlertsIconListener before you set the callback and NS_RELEASE it in the destroy callback.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Precisely my plan, yes. I'm just trying to think of a mochitest. I could probably test the alertfinished callback, but the alertclickcallback listener seems impossible to test due to the myriad of notification systems we now use.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Created an attachment (id=373274)
Callbacks 2

This includes a mochitest.

What I'm worried about is what happens if Growl is absent. Does the alerts service throw? If not, I'll have to update this patch to make an exception for non Windows and Linux platforms.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Created an attachment (id=373275)
Callback 2.1

Oops, that patch had some cruft in it.

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

You'll hit two cases with Growl:
1) if growl is not installed, the call to getService will fail (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/alerts/src/mac/nsAlertsService.mm#150)
2) if growl is not running, dispatching the notification will fail (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/alerts/src/mac/nsAlertsService.mm#97)

For the unit test boxes, you'll hit (1)

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

+ alertTimeout = setTimeout(killAndFail, ALERT_TIMEOUT);

Don't do this. Let the normal mochitest timeout mechanism trigger test failure.

+ nsAlertsIconListener* alert = (nsAlertsIconListener*) user_data;

static_cast

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Created an attachment (id=373592)
Callbacks 2.2

Fix review comments.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Wow, that was fast.

Revision history for this message
In , Alexander Sack (asac) wrote :

(In reply to comment #26)
> As a note, even Ubuntu's implementation supports actions, it's just not "as
> shiny" right now. We should let the caller decide whether they want a callback
> or not.

while this is true, it doesn't mean that those actions should be used.

In general, apps should not add actions if there is no "actions" capability announced by the server through:

/**
 * Returns the capabilities of the notification server.
 *
 * @return A list of capability strings. These strings must be freed.
 */
GList *notify_get_server_caps(void);

So what we can do is two things (or maybe both):

1. omit the actions in system alerts service if the there is no actions support available
2. add a supportsActions method to system alerts component to allow the caller to decide on its own what to do in case there are no actions available on system.

Revision history for this message
In , Dao (dao) wrote :

1) has already been discussed and is the reason this bug has been reopened. I don't see what you expect the caller to do in 2). Seems like we should avoid that too. Ideally we'd fall back to our cross-platform implementation if the notification server isn't capable of actions.

Revision history for this message
In , Alexander Sack (asac) wrote :

(In reply to comment #45)
> 1) has already been discussed and is the reason this bug has been reopened. I

1) might have been discussed, but it wasn't implemented (the implementation just dropped actions no matter what.

> don't see what you expect the caller to do in 2). Seems like we should avoid
> that too. Ideally we'd fall back to our cross-platform implementation if the
> notification server isn't capable of actions.

I think the alertsservice - which currently delegates to the system one - could do that automatic fallback; anyway, in order to decide this, the system alertsservice needs to exports that info first. Thats basically what i meant with 2). Other xulapps could then look up the system alerts service and do their own decision (e.g. if they don't use actions they can just happily use system notification even when no actions are supported)

Revision history for this message
In , Alexander Sack (asac) wrote :

1. Capabilities of notify-osd (shipped by ubuntu default):
'body', 'body-markup', 'icon-static', 'image/svg+xml', 'x-canonical-private-
synchronous', 'x-canonical-append', 'x-canonical-private-icon-only', 'x-canonical-truncation', 'private-synchronous', 'append', 'private-icon-only', 'truncation'

2. Capabilities of notification-daemon:
'actions', 'body', 'body-hyperlinks', 'body-markup', 'icon-static'

Revision history for this message
In , Dao (dao) wrote :

(In reply to comment #46)
> 1) might have been discussed, but it wasn't implemented (the implementation
> just dropped actions no matter what.

Right, but it's bad for the same reason. We can't drop callbacks like the original implementation did it, and we also can't do it depending on the notification server -- unless we care only about a know set of servers and are confident that they'll continue to support actions.

Revision history for this message
In , Alexander Sack (asac) wrote :

(In reply to comment #48)
> (In reply to comment #46)
> > 1) might have been discussed, but it wasn't implemented (the implementation
> > just dropped actions no matter what.
>
> Right, but it's bad for the same reason. We can't drop callbacks like the
> original implementation did it, and we also can't do it depending on the
> notification server -- unless we care only about a know set of servers and are
> confident that they'll continue to support actions.

but if the server does not announce "actions" capability adding them does not make it better :) ... anyway, i think the right way to move forward would be 2) as in comment 46

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

One thing we could do is provide the click callback immediately as the alert appears, if the server doesn't support actions. For example, the downloads window will appear when the downloads complete alert is also shown. That would be compatible with our current system but might be annoying in some circumstances.

Comment 46 sounds good in theory I suppose, but changing the interface as well as all its users (not to mention third party extensions!) sounds a little much.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

But that could probably be a topic of another bug, after this is checked in?

Revision history for this message
In , David-humphrey (david-humphrey) wrote :

> One thing we could do is provide the click callback immediately as the alert
> appears, if the server doesn't support actions. For example, the downloads
> window will appear when the downloads complete alert is also shown. That would
> be compatible with our current system but might be annoying in some
> circumstances.

+1 -- we really should have parity with other notification systems we use (e.g. Growl), so that all the existing "click" logic works here too.

Revision history for this message
In , Dao (dao) wrote :

(In reply to comment #50)
> One thing we could do is provide the click callback immediately as the alert
> appears, if the server doesn't support actions. For example, the downloads
> window will appear when the downloads complete alert is also shown. That would
> be compatible with our current system but might be annoying in some
> circumstances.

That seems to destroy the idea of lightweight notifications.

(In reply to comment #51)
> But that could probably be a topic of another bug, after this is checked in?

Well, you want to turn it on in this bug, right? Yet notify-osd doesn't seem to
support actions officially. That doesn't seem right to me.

Revision history for this message
In , Dao (dao) wrote :

Specifically, we use notifications because we don't want to steal focus.

Revision history for this message
In , Alexander Sack (asac) wrote :

(In reply to comment #50)
> Comment 46 sounds good in theory I suppose, but changing the interface as well
> as all its users (not to mention third party extensions!) sounds a little much.

This was not exactly the idea I had in mind. AFAIK, currently alertsservice is the only place that decides whether to delegate to system service or not. ATM this decision only depends on whether the component is available or not.

We could extend that check to also consider whether actions are available; if there is no actions support, it would fall back to the xul implemention - thus, preserving the old contract.

However, toolkit apps/extensions that are happy with the actionless approach could explicitly opt-out from that by using a different method and/or by directly looking up/using the system-alerts component.

Revision history for this message
In , Alexander Sack (asac) wrote :

(In reply to comment #51)
> But that could probably be a topic of another bug, after this is checked in?

The current patch should at least not add actions when the server doesn't announce "actions" capability. That would also be in line with comment 25.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

(In reply to comment #56)
> (In reply to comment #51)
> > But that could probably be a topic of another bug, after this is checked in?
>
> The current patch should at least not add actions when the server doesn't
> announce "actions" capability. That would also be in line with comment 25.

I'd rather do this than fallback to XUL, this bug seems like a giant waste if the most popular distribution doesn't benefit from it in any way.

Revision history for this message
In , Jakub 'Livio' Rusinek (liviopl-pl) wrote :

> this bug seems like a giant waste if
> the most popular distribution doesn't benefit from it in any way.

What does this mean? Lack of unity in question of notification (notify-osd, notification-daemon) caused, mostly by Ubuntu, is the problem?

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

(In reply to comment #58)
> > this bug seems like a giant waste if
> > the most popular distribution doesn't benefit from it in any way.
>
> What does this mean? Lack of unity in question of notification (notify-osd,
> notification-daemon) caused, mostly by Ubuntu, is the problem?

Sort of, yes. I don't see how this can be resolved on our side if the notification server doesn't support actions, but I also want to kill that ugly popup for as many people as humanly possible, and adopt the native library.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

I suppose I can just not add the action callback if the server doesn't support them.

Revision history for this message
In , Dao (dao) wrote :

(In reply to comment #57)
> > The current patch should at least not add actions when the server doesn't
> > announce "actions" capability. That would also be in line with comment 25.
>
> I'd rather do this than fallback to XUL, this bug seems like a giant waste if
> the most popular distribution doesn't benefit from it in any way.

I'm not sure that I agree with comment 25 there, or maybe I misunderstand it. If the caller wants a callback (which might even be reflected in the notification message) and the server tells us that it doesn't support it, falling back to XUL is the only sane solution, IMHO. I think this needs to be evangelized on the Ubuntu side.

Revision history for this message
In , Jakub 'Livio' Rusinek (liviopl-pl) wrote :

> I think this needs to be evangelized on the Ubuntu side.

Talk to Mark Shuttleworth. It was his idea to make _another_ notification daemon, but with less possibilities (completely without actions).

Revision history for this message
In , Alexander Sack (asac) wrote :

(In reply to comment #60)
> I suppose I can just not add the action callback if the server doesn't support
> them.

So:

#define NOTIFY_CAPS_ACTIONS_KEY "actions"

GList *server_caps = notify_get_server_caps();
if (g_list_find (server_caps, NOTIFY_CAPS_ACTIONS_KEY))
...

if you do this, consider to cache the result somehow as you would otherwise do another dbus call for each notification.

Revision history for this message
In , Mike Connor (mconnor) wrote :

We should absolutely fall back to XUL if there's no actions support, IMO. I think the Ubuntu system is unnecessarily restrictive, and the design guidelines actually are more damaging than helpful. I guess I need to write up that critique after all...

Let's do the sane thing, that Windows and OS X (via Growl) already do, and we can work to convince Ubuntu of the error of their ways... :)

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Created an attachment (id=375563)
Callbacks 3

This falls back to XUL if the server doesn't support actions.

Revision history for this message
In , Dao (dao) wrote :

I tried to land this (http://hg.mozilla.org/mozilla-central/rev/99cdecb3734f), but will have to back it out because of this:

checking for libnotify >= 0.4... Package libnotify was not found in the pkg-config search path. Perhaps you should add the directory containing `libnotify.pc' to the PKG_CONFIG_PATH environment variable No package 'libnotify' found
configure: error: Library requirements (libnotify >= 0.4) not met; consider adjusting the PKG_CONFIG_PATH environment variable if your libraries are in a nonstandard prefix so pkg-config can find them.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1241424892.1241424932.5678.gz

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Er, I remember libnotify dev headers being installed on mozilla-central, I'll take another look at the bug.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Bug 473831 was marked fixed, but they mention only i386 packages.
The red only occurred on the 64-bit box, but the builds that happened around the same time on the 32-bit boxes still seem to be going.

Revision history for this message
In , Nrthomas (nrthomas) wrote :

linux64 slave now has a libnotify-devel installed, previously only had libnotify. Details in bug 473831 comment #9.

Revision history for this message
In , Ventnor-bugzilla (ventnor-bugzilla) wrote :

Wonderful, thanks.

Revision history for this message
In , Dao (dao) wrote :

http://hg.mozilla.org/mozilla-central/rev/c2ba27e9e639

For some reason, Linux x86-64 mozilla-central hasn't started building yet...

Revision history for this message
Paul Sladen (sladen) wrote :

Binary package hint: notify-osd

Firefox currently provides its own notifications, such as "All downloads have finished". Although similar in style to notify-ods' messages popup-box they are completely separate and unique to Firefox.

Ideally this could be integrated to use notify-ods.

Changed in firefox (Ubuntu):
importance: Undecided → Wishlist
Changed in notify-osd (Ubuntu):
importance: Undecided → Medium
importance: Medium → Wishlist
Revision history for this message
Micah Gersten (micahg) wrote :

AFAIK, no changes need to be made in notify-osd for this.

Changed in notify-osd (Ubuntu):
status: New → Invalid
Revision history for this message
Micah Gersten (micahg) wrote :

This already appears to be fixed upstream and should work in Firefox 3.6. Can you test the daily build to see if this is the case?
https://launchpad.net/~ubuntu-mozilla-daily/+archive/ppa
Thanks.

affects: firefox (Ubuntu) → firefox-3.5 (Ubuntu)
Changed in firefox-3.5 (Ubuntu):
status: New → Triaged
Changed in firefox:
milestone: none → 3.6
Changed in firefox:
status: Unknown → Fix Released
Revision history for this message
Micah Gersten (micahg) wrote :

I was mistaken, this was added upstream but not such that it'll work with Ubuntu. We still need to work on this.

Revision history for this message
Paul Sladen (sladen) wrote :

Micah: my reading of upstream's intentions was to check the capabilities offered, and explicitly disable the libnotify if the link/feedback capability was not present (eg. -osd).

Revision history for this message
Micah Gersten (micahg) wrote :

firefox will be the source package for Firefox 3.6, so adding this task

Changed in firefox (Ubuntu):
importance: Undecided → Wishlist
status: New → Triaged
Revision history for this message
Micah Gersten (micahg) wrote :

Reopening notify-osd task as Mozilla only went so far in their implementation

Changed in notify-osd (Ubuntu):
status: Invalid → New
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (9.6 KiB)

This bug was fixed in the package firefox - 3.6+nobinonly-0ubuntu1

---------------
firefox (3.6+nobinonly-0ubuntu1) lucid; urgency=low

  * New upstream release v3.6 (FIREFOX_3_6_RELEASE)
    + fix LP: #449744 - Firefox crashes when attempting to load Firebug 1.5
    + fix LP: #66015 - Duplicate spell checking dictionaries for every entry
    + fix LP: #132938 - tooltips dont work in sidebar
    + fix LP: #195698 - Password asked separately for each tab that requires it
                        (proxy)
    + fix LP: #239462 - tooltips disappear too fast
    + fix LP: #385816 - Resize corner grab stays visible after maximize
    + fix LP: #429476 - firefox crash on javascript page
    + fix LP: #432876 - Icons missing in Firefox searchbox drop down list
    + fix LP: #486284 - maxlength on input box can be overriden by autocomplete
    + fix LP: #501393 - Integrate Firefox notifications with notify-osd bling

  [ H. Montoliu <email address hidden> ]
  * fix LP: #361052 - firefox apport hook fails to retrieve pluginreg.dat file
  * update debian/apport/firefox-3.6.py - removed unused code and minor refactoring.

  [ Fabien Tassin <email address hidden> ]
  * Update the location of the upsteam branch now that 3.6/Namoroka has its own
    branch, and trunk moved on to 3.7
    - update debian/mozclient/firefox-3.6.conf
  * Use Namoroka instead of Shiretoko as brand name and use it for snapshots.
    Name it Namoroka in the Preferred Application UI too
    - update debian/firefox-3.6-shiretoko.desktop => debian/firefox-3.6-namoroka.desktop
    - update debian/firefox-3.6.xml
    - update debian/rules
  * Target the 'default' branch instead of tip
    - add debian/moz-rev.sh
    - update debian/mozclient/firefox-3.6.conf
  * Add firefox 3.6 to the list of Preferred Applications in Gnome
    - add debian/firefox-3.6.xml
    - update debian/firefox-3.6-gnome-support.install
  * Add ${misc:Depends} to all non-transitional packages, make firefox-3.6-dbg
    depend on firefox-3.6 with the exact same version, move -dbg packges to
    priority extra and add firefox-3.6-gnome-support-dbg
    - update debian/control
  * Update diverged patches:
    - update debian/patches/browser_branding.patch
    - update debian/patches/firefox-profilename
    - update debian/patches/ubuntu_bookmarks.patch
    - update debian/patches/lp185622_system_path_default_browser.patch
    - update debian/patches/dont_depend_on_nspr_sources.patch

  [ Alexander Sack <email address hidden> ]
  * add libnotify-dev to build-depends
    - update debian/control
  * add libiw-dev to build-depends to fix build failure
    - update debian/control
  * until we move searchplugins to a separate package provided only by the current default
    firefox, we need to make firefox-3.6 replace all the older firefox binary packages:
    firefox-3.5, firefox-3.2, firefox-3.1, firefox-3.0
    - update debian/control
  * implement MIN_SYS_DEPS approach that does not use system xulrunner
    and only a minimal set of system dependencies.
    + drop patches not required anymore:
      - delete debian/patches/dont_depend_on_nspr_sources.patch
      - update debian/patches/series
    + update browser directory provider...

Read more...

Changed in firefox (Ubuntu):
status: Triaged → Fix Released
mercedes (preciousa1237)
Changed in firefox (Ubuntu):
status: Fix Released → Confirmed
Revision history for this message
Draycen DeCator (ddecator) wrote :

@mercedes
Please do not change the status of anything without explaining why you are doing so.

Changed in firefox (Ubuntu):
status: Confirmed → Fix Released
Revision history for this message
Jacopo Moronato (jmoronat) wrote :

It is not fixed for me.

Revision history for this message
Guillaume Pascal (guigui14100-deactivatedaccount) wrote :

Not fixed for me

Revision history for this message
Micah Gersten (micahg) wrote : Re: [Bug 501393] Re: Integrate Firefox notifications with notify-osd bling

Support for notify-osd is now included in Firefox 3.6, we still need to
actually make it work though.

On 03/10/2010 12:19 PM, guigui14100 wrote:
> Not fixed for me
>
>

Revision history for this message
Micah Gersten (micahg) wrote :

Correction...support for libnotify is now in Firefox 3.6.

On 03/10/2010 12:27 PM, Micah Gersten wrote:
> Support for notify-osd is now included in Firefox 3.6, we still need to
> actually make it work though.
>
> On 03/10/2010 12:19 PM, guigui14100 wrote:
>
>> Not fixed for me
>>
>>
>>
>

Revision history for this message
Matthew Paul Thomas (mpt) wrote :

I think this bug report is invalid.

The Mozilla bug report linked to this one is about libnotify integration in general, not about Notify OSD integration in particular. As described in the 2009-05-03 comment, "This falls back to XUL if the server doesn't support actions." So Firefox asks the notification server whether it allows interactive notifications. Notify OSD says "Thanks for asking, but no, I don't". Firefox then goes "Okay then", and uses its own interactive notification system instead.

This is all as it should be. If Firefox didn't check with the notification server first, every completed download would produce an ugly alert box from Notify OSD. If Firefox sent a non-interactive notification to Notify OSD, it would be quite frustrating because it wouldn't let you open the downloaded file. And if Notify OSD allowed interactive notifications in general, we'd be causing precisely the kind of accidents that we created Notify OSD to avoid.

Now, it would be nice if Firefox's download-complete notifications were more elegant and less error-prone than they are. One way to do this (which I suggested to the Firefox lead developer a year ago) would be for the existing Downloads window to request attention. Another way would be for the notifications to be embedded in the browser window, as they are in Chromium. Neither of those are anything to do with Notify OSD; it's not the right tool for the job.

Revision history for this message
Benjamin Drung (bdrung) wrote :

You can install xul-ext-notify as workaround.

Revision history for this message
YannUbuntu (yannubuntu) wrote :

why not installing xul-ext-notify package by default ?

Revision history for this message
In , YannUbuntu (yannubuntu) wrote :

Dear all,
according to https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/610362 , Firefox's notifications don't work with Ubuntu's implementation.

is there something mozilla can do to help solving this ?

Revision history for this message
In , Dao (dao) wrote :

(In reply to comment #72)
> Dear all,
> according to https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/610362 ,
> Firefox's notifications don't work with Ubuntu's implementation.
>
> is there something mozilla can do to help solving this ?

Please read through this bug ... We need action callback support for our notifications.

Revision history for this message
YannUbuntu (yannubuntu) wrote :

3 ideas :
- Ubuntu displays Firefox notifications in Notify OSD (so removes the action callback in exchange of a nicer notification). xul-ext-notify package seems to allow this.
- Ubuntu creates a 2nd notification system, complementary to Notify OSD, which provides action callback support and would be embedded in the browser window
- Firefox provides a way to customize/skin the appearance of its notifications, in order to better integrate them in Ubuntu.

Changed in firefox:
importance: Unknown → Medium
Revision history for this message
Danillo (danillo) wrote :

The simplest workaround for this issue (the package xul-ext-notify) was in the Ubuntu repositories until Maverick, but it didn't make it to Natty. Is there any chance it will be packaged for Oneiric?

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in notify-osd (Ubuntu):
status: New → Confirmed
Revision history for this message
Adolfo Jayme Barrientos (fitojb) wrote :

I agree with mpt. notify-osd notifications are designed to be non-interactive.

no longer affects: firefox-3.5 (Ubuntu)
Changed in notify-osd (Ubuntu):
status: Confirmed → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.