Closed Bug 1151843 Opened 5 years ago Closed 5 years ago

[Messages][Refactoring] Use notification "data" attribute instead of icon URL query string parameters

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect, minor)

ARM
Gonk (Firefox OS)
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: azasypkin, Assigned: azasypkin)

References

Details

Attachments

(1 file)

After patch for bug 899585, notification options have dedicated "data" attribute to associate custom data with notification object. So we should stop abusing icon URL attribute to store custom data and start using "data" instead.
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Comment on attachment 8589252 [details] [review]
[gaia] azasypkin:bug-1151843-notification-data > mozilla-b2g:master

Hey Julien, could you please review this patch?

I've split it into two commits, 2nd one is just further improvement of onNotification handler, I'm still not sure if it worth creating separate bug for it, but let me know what you think.

Thanks!
Attachment #8589252 - Flags: review?(felash)
Comment on attachment 8589252 [details] [review]
[gaia] azasypkin:bug-1151843-notification-data > mozilla-b2g:master

r=me for the first commit only.

Please handle the comment about the shared Notification mock if you think it's good and it's not breaking other tests, but I'm not blocking on this.
Attachment #8589252 - Flags: review?(felash) → review+
See Also: → 1153784
Comment on attachment 8589252 [details] [review]
[gaia] azasypkin:bug-1151843-notification-data > mozilla-b2g:master

Hey Michael,

In this patch I modified  shared/test/unit/mocks/mock_notification.js a bit, could you please let me know if it looks OK for you?

Thanks!
Attachment #8589252 - Flags: review?(mhenretty)
Comment on attachment 8589252 [details] [review]
[gaia] azasypkin:bug-1151843-notification-data > mozilla-b2g:master

Looks fine, thanks!
Attachment #8589252 - Flags: review?(mhenretty) → review+
Thanks for review guys!
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.