903547 - Change property names for notifications in gaia to match new Notifications spec

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Bug 900279 didn't fix up gaia notifications all the way. The property names in addSpecifications don't match the spec (uses id instead of tag, bidi instead of direction, etc...). In order to be able to use the new notifications API and have things show up properly, these need to match. This will also require changes in apps that call addNotification directly, all of which live in the system app (voicemail, cell broadcast, etc...).

Comment 1

[Candice Serran (:cserran)]

Candice Serran changed story state to started in Pivotal Tracker

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - WIP: Change gaia notification properties to match notification spec

Changed the following properties to match the notification spec names, so that when a notification from the new API is passed in, it should work:

- text -> body
- bidi -> dir
- id -> tag

Also changed all tests to match these names. Tests run and pass.

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok. This bug may actually be invalid. Having finally traced notifications all the way from the dom code to gaia, I think I see what's going on now.

Notifications only have the name "notifications" in terms of the DOM API and the system message they use ("desktop-notification"). They're actually shipped through the alert service. This is where the bidi/text/id language came from, since it matches the function calls in nsIAlertsService.idl.

For FxOS, AlertsService is implemented in the b2g specific portions of gecko (b2g/components/AlertsService.js which then sends to b2g/chrome/shell.js for sending system messages to gaia), which continues the use of the terminology up through the NotificationsScreen in the system app in gaia (which receives the system messages from gecko), so even though it's called notifications again there, it's actually still based on the AlertsService naming. Through this whole chain, none of the data has been lost, it's just changed names and been handed off multiple times.

Any app calling new Notification(...) (the way to get notifications via the new API) will get routed through this system, so the right thing should already happen in the end without the name changes I thought were needed due to thinking the spec was dictating the full layout of the system.

If this /is/ the case, then the code wchen landed in gaia already should do replacements via id (née tag) similarities, the notification object will support close, meaning this bug is invalid, bug 892530 is already finished, and bug 892528 simply needs a button added to the UI test. Once I can confirm I have everything correct here, I'll update all bugs accordingly.

Comment 4

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

fb?'ing wchen to see if I have everything straight in comment 3, and hopefully clearing up some of the confusion I had when talking to him about this earlier today. :)

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok. On discussion with mhenretty and jsmith, looks like my assumptions in comment 3 are correct, marking invalid.

Comment 6

[Candice Serran (:cserran)]

Kyle Machulis changed story state to finished in Pivotal Tracker

Comment 7

[Candice Serran (:cserran)]

Kyle Machulis changed story state to delivered in Pivotal Tracker

Comment 8

[Candice Serran (:cserran)]

Gregor Wagner changed story state to accepted in Pivotal Tracker