1. Title: The handset is clearing all the notifications when click on one notification 2. Precondition: Have several long SMS/MMS threads. SMS delivery reports ON. 3. Tester's Action: 1. Receive some messages 2. Check notifications bar 3. Select one message notification 4.Check the notifications bar again 4. Detailed Symptom (ENG.) : The handset is clearing all the notifications 5. Expected: The handset should clear only the notification previously checked 6. Reproducibility: Y 1) Frequency Rate : 100% 7. Gaia Master/v1-train: Reproduced on v1-train 8. Gaia Revision: 613e69ee0a009399130ad2cbb8b9d0462cd1fc70 9. Personal email id: sasikala.paruchuri8@gmail.com

Some investigation. Precondition: have 2 sms messages unread on notification. When 1st message is tapped, the tab function is called. tap: function ns_tap(notificationNode) { window.dispatchEvent(event); ... this.removeNotification(notificationNode.dataset.notificationID, false); This launches the sms app and removes the 1st notification. Then when app is launched, the 2nd notification is removed by 'appopen' event. window.addEventListener('appopen', this.handleAppopen.bind(this)); handleAppopen: function ns_handleAppopen(evt) { var manifestURL = evt.detail.manifestURL, selector = '[data-manifest-u-r-l="' + manifestURL + '"]'; To Alive: Would there be an easy fix for this? I'm not sure how you'll like having 'if app == sms, do nothing' in handleAppopen.

Julien could you take a look as well? Thanks!

This is done on purpose in 1.1, sorry we won't change this.

If "all the notifications" that are cleared are from the SMS app, then i believe this is not a bug but a current behaviour. As the SMS app will clear all its related notifications. If notifications from other apps are also cleared, then this would be a bug, please renom leo? if this is the case.

To be clear (once again), since we don't have access to the "new" Notification API that is implemented in mozilla central, but not in b2g18, there are some use cases that we can't do. In particular we can't precisely control the notifications from the app. So we made a number of simplifications and this is what we're doing in 1.1: * when an app is displayed, either: + launched directly + launched from a notification + is fullscreen when coming out of lockscreen we remove all the notifications that this app has sent. Note: I'm not sure if the app is launched with an activity; I think we're removing the notification too in that case. This is not perfect, but I think this is the best compromise we can do with the current tools we have. If you want more, we need to uplift bug 782211 and do some work in system and the gecko glue to properly support the new notification mechanism and do work in the Sms app. I don't think we want to do this now. Thus renominating. Please put koi? if it's not leo+ again, as I definitely want to improve this in the next version.

This is now koi? as agreed by leo partners.

Clear ni? since unnecessary now.

Bugmorphing this to a System bug. We implemented workarounds in System and Gecko to remove all notifications for an app when that app : * is launched * is displayed * is toplevel when we go out of the lockscreen This was bug 824549 and bug 868816. Before removing the workarounds in the system, all apps that uses notifications will need to be changed to handle their own notifications if necessary. SMS counterpart will be done in bug 855165, I don't know if other bugs exist for other apps already.

Not blocking - not critical to 1.2 feature set.

(In reply to Julien Wajsberg [:julienw] from comment #9) > Bugmorphing this to a System bug. > > We implemented workarounds in System and Gecko to remove all notifications > for an app when that app : > * is launched > * is displayed > * is toplevel when we go out of the lockscreen > > This was bug 824549 and bug 868816. > > Before removing the workarounds in the system, all apps that uses > notifications will need to be changed to handle their own notifications if > necessary. > > SMS counterpart will be done in bug 855165, I don't know if other bugs exist > for other apps already. I don't know if it is the current behavior, but I think that the System should trigger a 'applaunch' event. Is it the current behavior?

Caio, I don't understand what you want to say here :) You should have a look to the patches of the two bugs.

Okay so

As far as I could understand, both bug 929895 (koi+) and bug 926318 (v1.3+) are manifestations of the workaround that this bugs aims at removing.

Not a blocker, but feel free to keep working on this.

this blocks a 1.3+, 1.3?

(In reply to Joe Cheng [:jcheng] from comment #17) > this blocks a 1.3+, 1.3? This is not a blocker - we've discussed this multiple times in triage that this is a feature we should definitely work on, but won't block 1.3. The solution that should be done to resolve the blocking need on bug 926318 should involve blacklisting the dialer app, as that's the current workaround we're suggesting for any blocking bugs related to this problem. This is on product's backlog for a future release, so we will consider this feature at some point.

The more I think of it, the more I think we should not remove tapped notifications for new-style notifications. Only the Notification API clients should remove their notifications, the System app should not touch them.

After looking the code everywhere and thinking about it for two days, and discussing with julienw about this, I think the plan should be: - remove the hacks in system app - change notification helper to use the new API - implement a "simple" interface in notification helper that does the same minimum as in the old API and mimic the behavior: taping removes the notif from the helper directly - move all apps to the new simple interface (and this should be trivial) - convert each app after to make use of the new API

Sorry for the noise.

this is blocking bug 926318, 1.3?

I'd be happier with removing 1.3+ on bug 926318. I know we moved this a lot already, but now Alexandre is _really_ working on it these days, so I'm confident it will be implemented correctly for 1.4.

Yeah, this is definitely not a blocker for 1.3.

We definitively can't remove the hacks in system app 'like this'. Until the API is not unsable anymore, and after discussing with julienw, we should keep the old behavior for apps that have the old API. We should be able to distinguish the API from Gecko events. As far as I've checked for now, the 'id' is generated differently depending on the API, so we could use this.

id field of the mozChromeEvent in the system app for a notification sent through the new API: "id":"app://uitest.gaiamobile.org/manifest.webapp#notag:{45a6fffc-da7b-440e-8cf6-088fc4645cb6}" and in the case of the old API: "id":"app-notif-{490fdc61-89dc-47c2-8701-4086aabcd819}" This is being set by gecko at https://hg.mozilla.org/mozilla-central/file/9f12a9fab080/b2g/components/AlertsService.js#l70

Please find attached a PR that detects whether a notification was sent through the old API or through the new one, and disable the hacks in the case of the new API. This is needed for bug 910999.

I'm waiting green travis at https://travis-ci.org/mozilla-b2g/gaia/builds/15587037

Looks like it got stalled :(, triggering a new one.

Travis is green now.

https://github.com/lissyx/gaia/commit/b92445c187396445aed6a657bf9e037d988bf5d6 I'm not marking this FIXED yet, the workaround is still in place until we remove and do not support mozNotification anymore.

I think you should resolve/fix it and file a new bug for mozNotification (and make it depends on another bug to drop support for mozNotification in B2G).

(I doubt this will happen soon ;) )

Followup bug 952454 is about ditching those workarounds and API version detection, bug 952453 is about total diappearance of mozNotification API.

Comment on attachment 8348187 [details] Link to Github https://github.com/mozilla-b2g/gaia/pull/14719 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): [User impact] if declined: improper notification removal: notifications sent with the new API are expected to be programmatically closed, not closed when tapped by user [Testing completed]: this has been in gaia for a couple of weeks, working on all devices [Risk to taking this patch] (and alternatives if risky): low, we are only impacting applications using the new Notification API [String changes made]: none

Comment on attachment 8348187 [details] Link to Github https://github.com/mozilla-b2g/gaia/pull/14719 a- - see the blocking bug for more details.

This is a 1.3 uplift PR.

Comment on attachment 8390757 [details] Github PR, 1.3 uplift NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): verification problem in bug 910999 [User impact] if declined: Notification API for voicemail doesn't work. (cert blocker) [Testing completed]: on trunk for a while and locally [Risk to taking this patch] (and alternatives if risky): [String changes made]:

Too risky for 1.3 - we've already found blocking regressions from this. We need a contextual solution here.

(In reply to Jason Smith [:jsmith] from comment #40) > Too risky for 1.3 - we've already found blocking regressions from this. We > need a contextual solution here. Apparently the regression concern I had wasn't right. According to Gregor, we've got no alternative here, so I'm resetting the blocking flag.

v1.3: https://github.com/mozilla-b2g/gaia/commit/961a6a261d2fdb5e06e3d05e4b47a65eaf0ea910

Wow uplifting this with all other follow-ups gives me the creeps. I wonder if a hacky hack wouldn't have been possible to fix the 1.3+ bug. *crossing fingers*