Fennec implemented an AlertClose event, fired when notification boxes are closed. It would be nice to have it in toolkit instead of mb so that all our apps can use it (they already have an AlertOpen event).

I second this. And instead of stating "New in Mobile 2" on page https://developer.mozilla.org/en/XUL/notificationbox it's more helpful to state "Only available on Mobile 2".

I found this bug while researching a patch I'm working on for Bug 752988 - Chrome Scratchpad focus bug ... For the solution there, I'm having to add an AlertClose to the toolkit/notification.xml button ... how much of this bug does that change satisfy?

I added the new AlertClose event to the notification box à la Fennec, and a quickie mochitest for both AlertActive and AlertClose events. Can you give this a look?

Comment on attachment 647436 [details] [diff] [review] Patch (v1) I don't understand why we should add this. The event appears to be unused in Fennec, so it seems that we should just remove it there.

Comment on attachment 647436 [details] [diff] [review] Patch (v1) Yes I see. The AlertClose event was fired and listened for in Bug 509408 - when getting notification asking to allow popup, it covers the url bar ... The EventListener was removed somewhere along the line, but the dispatch was left hanging. I'm asking :wesj in Fennec if I can remove it for code clean up and to complete this bug, as he was involved here back when.

We don't use notifications.xml in Native Fennec anymore. I don't think doing code cleanup on XUL Fennec is worth it. We risk breaking something and having no idea we broke it because no one is using/testing it anymore. Note Metro Firefox is also still using this code though: https://mxr.mozilla.org/projects-central/source/elm/browser/metro/base/content/bindings/notification.xml#81 Removing it there might be better cleanup. Sorry to keep bumping you around :(

Interesting! I've never seen that code area before "This is the home for incomplete mozilla-central projects." ... Maybe this patch should just be WONTFIX'ed, as Widgets doesn't want to add the AlertClose, and Fennec doesn't want to drop their AlertActive ?

Sorry, "Fennec / doesn't want to drop their AlertClose."

Dropping off this one until further clarification.

Pushing this over to metro firefox and cc'ing bbondy just so you guys know this code is there... doing nothing.

Adding Matt since he's back in this code!

I am interested in AlertClose so I can write mochitests against global-notificationbox for FHR. IMO an explicit event is much more useful than sniffing lower-level DOM events. Although poking around, it does appear there is an undocumented "Removed" event in notification.xml:169. Maybe this is part of some DOM event magic. I don't know DOM that well. I dunno.

"removed" isn't a DOM event. It's just a call to a function that you may add as appendNotification's last argument.

This is no longer unused on Metro.

I don't understand comment 14. http://mxr.mozilla.org/mozilla-central/search?string=%22AlertClose%22 shows that AlertClose is still around. Even if it wasn't, wouldn't that make this bug FIXED/WORKSFORME?

(In reply to Dão Gottwald [:dao] from comment #15) > I don't understand comment 14. > http://mxr.mozilla.org/mozilla-central/search?string=%22AlertClose%22 shows > that AlertClose is still around. Even if it wasn't, wouldn't that make this > bug FIXED/WORKSFORME? What I said in comment 14 is that it is "no longer unused" -- i.e. it is still around, but we actually use it now, so we don't want to remove it. I guess INVALID is a better resolution.