Closed
Bug 1100876
Opened 10 years ago
Closed 9 years ago
Do not resend notifications that we should not resend
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Firefox OS Graveyard
Gaia::System
Tracking
(Not tracked)
RESOLVED
FIXED
2.2 S1 (5dec)
People
(Reporter: gerard-majax, Assigned: gerard-majax)
References
Details
(Whiteboard: [systemsfe])
Attachments
(1 file, 3 obsolete files)
3.41 KB,
patch
|
gerard-majax
:
review+
smaug
:
review+
|
Details | Diff | Splinter Review |
Let's leverage the mozbehavior dict and implement this properly. We will be able to cleanup some code (Voicemail for instance).
Assignee | ||
Comment 1•10 years ago
|
||
There are some notifications that we know we don't want to resend. For instance, Voicemail notifications are some, since everytime the network will be ready it will notify us of the status, so we don't have to resend it on reboot. We add a 'resend' key in the MozBehavior dict for this usecase.
Assignee | ||
Comment 2•10 years ago
|
||
Without the fix: > 60 INFO ::Notification Tests:: | Trying to resend non-resendable notification > 61 INFO TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/notification/test_notification_resend.html | One notification not resent - got 1, expected 0 With the fix: > 60 INFO ::Notification Tests:: | Trying to resend non-resendable notification > 61 INFO TEST-PASS | /tests/dom/tests/mochitest/notification/test_notification_resend.html | One notification not resent
Assignee | ||
Comment 3•10 years ago
|
||
Pending try: https://tbpl.mozilla.org/?tree=Try&rev=39a2d6ef7f79
Assignee | ||
Updated•10 years ago
|
Attachment #8525327 -
Flags: review?(mhenretty)
Updated•10 years ago
|
Whiteboard: [systemsfe]
Target Milestone: --- → 2.2 S1 (5dec)
Comment 4•10 years ago
|
||
Comment on attachment 8525327 [details] [diff] [review] Add noresend to MozBehavior dict r=mhenretty Review of attachment 8525327 [details] [diff] [review]: ----------------------------------------------------------------- Awesome! I flashed this on a phone, and updated apps/system/js/voicemail.js to specify noresend for its notifications, and bug 1088224 was indeed fixed!
Attachment #8525327 -
Flags: review?(mhenretty) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 5•9 years ago
|
||
need dom peer review: remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
Flags: needinfo?(lissyx+mozillians)
Keywords: checkin-needed
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8525327 [details] [diff] [review] Add noresend to MozBehavior dict r=mhenretty Olli, if you can check the webidl part :)
Flags: needinfo?(lissyx+mozillians)
Attachment #8525327 -
Flags: review?(bugs)
Assignee | ||
Comment 7•9 years ago
|
||
There are some notifications that we know we don't want to resend. For instance, Voicemail notifications are some, since everytime the network will be ready it will notify us of the status, so we don't have to resend it on reboot. We add a 'resend' key in the MozBehavior dict for this usecase.
Assignee | ||
Updated•9 years ago
|
Attachment #8525327 -
Attachment is obsolete: true
Attachment #8525327 -
Flags: review?(bugs)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8528286 [details] [diff] [review] Add noresend to MozBehavior dict r=mhenretty,smaug Carrying r+
Attachment #8528286 -
Flags: review+
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8528286 [details] [diff] [review] Add noresend to MozBehavior dict r=mhenretty,smaug Asking review for the WebIDL change
Attachment #8528286 -
Flags: review?(bugs)
Comment 10•9 years ago
|
||
Comment on attachment 8528286 [details] [diff] [review] Add noresend to MozBehavior dict r=mhenretty,smaug Curious, do we have a bug open to move vibrationPattern to NotificationOptions and rename it to vibrate (to follow the spec). What does noresend mean in this context? Notifications aren't "sent" to anywhere.
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #10) > Comment on attachment 8528286 [details] [diff] [review] > Add noresend to MozBehavior dict r=mhenretty,smaug > > Curious, do we have a bug open to move vibrationPattern to > NotificationOptions > and rename it to vibrate (to follow the spec). > > > What does noresend mean in this context? Notifications aren't "sent" to > anywhere. The noresend here controls the behavior of the mozChromeNotifications.mozResendAllNotifications() behavior. This is used in the Gaia, in the System application, to be able to redisplay stored notifications.
Comment 12•9 years ago
|
||
Those sound like implementation details. What functionality does noresend represent? Is it like "don't re-show the notification" (though, the "re-" part is not clear to me.) Or is "notifyOnlyOnce" more correct? Or is the flag about persistency somehow?
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #12) > Those sound like implementation details. > What functionality does noresend represent? > Is it like "don't re-show the notification" (though, the "re-" part is not > clear to me.) That's it. What is not clear with the "re-" part?
Comment 14•9 years ago
|
||
Comment on attachment 8528286 [details] [diff] [review] Add noresend to MozBehavior dict r=mhenretty,smaug so, noresend doesn't really mean anything to me in this context. Notifications aren't sent anywhere. They are shown. So, showOnlyOnce ?
Attachment #8528286 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 15•9 years ago
|
||
There are some notifications that we know we don't want to resend. For instance, Voicemail notifications are some, since everytime the network will be ready it will notify us of the status, so we don't have to resend it on reboot. We add a 'resend' key in the MozBehavior dict for this usecase.
Assignee | ||
Comment 16•9 years ago
|
||
There are some notifications that we know we don't want to resend. For instance, Voicemail notifications are some, since everytime the network will be ready it will notify us of the status, so we don't have to resend it on reboot. We add a 'showOnlyOnce' key in the MozBehavior dict for this usecase.
Assignee | ||
Updated•9 years ago
|
Attachment #8528286 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8528414 -
Attachment is obsolete: true
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8528415 [details] [diff] [review] Add showOnlyOnce to MozBehavior dict r=mhenretty,smaug Carrying r+
Attachment #8528415 -
Flags: review+
Assignee | ||
Comment 18•9 years ago
|
||
Comment on attachment 8528415 [details] [diff] [review] Add showOnlyOnce to MozBehavior dict r=mhenretty,smaug Changed from noresend to showOnlyOnce
Attachment #8528415 -
Flags: review?(bugs)
Assignee | ||
Comment 19•9 years ago
|
||
New try: https://tbpl.mozilla.org/?tree=Try&rev=7519c6ed7e4a
Updated•9 years ago
|
Attachment #8528415 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 20•9 years ago
|
||
(In reply to Alexandre LISSY :gerard-majax from comment #19) > New try: https://tbpl.mozilla.org/?tree=Try&rev=7519c6ed7e4a Need to check that this one is green instead. https://tbpl.mozilla.org/?tree=Try&rev=3d6d3f4f8c23
Keywords: checkin-needed
Comment 21•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/9155e0c35233
Flags: in-testsuite+
Keywords: checkin-needed
Comment 22•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9155e0c35233
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•