1033933 - Notification.get() returns notifications for cleared notifications if app is killed

Status: RESOLVED FIXED

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

Description

[James Burke [:jrburke]]

Attachment: notifyget.zip - app that demonstrates the issue

The attached notifyget.zip contains an app that can be installed via the app manager to test. Unzip it in a "notifyget" directory.

Found on a Flame using a pvt build:

Gaia      85e97290431ce6aa0a965421e84d6070cd899129
Gecko     https://hg.mozilla.org/mozilla-central/rev/7075808c3306
BuildID   20140702040207
Version   33.0a1
ro.build.version.incremental=109
ro.build.date=Mon Jun 16 16:51:29 CST 2014

that then had a custom built gaia from today placed on it.

STR:

1) launch the notifyget app

2) Tap "Get Notifications and Add number to it" 3 times. The Notification should increase the count 3. In the adb logcat the following log statements will appear:

    NOTIFYGET TOTAL COUNT: 1
    NOTIFYGET TOTAL COUNT: 2
    NOTIFYGET TOTAL COUNT: 3

3) Tap the "Trigger app to close notification". The logcat will show:

    CLOSING notification with tag: notifyget

4) Kill the app, and relaunch it

5) Tap "Get Notifications and Add number to it" once. The logcat and notification will report a count of 2:

    NOTIFYGET TOTAL COUNT: 2

This is unexpected. It should have a count of 1.

Furthermore, tapping "Trigger app to close notification" will report closing 2 notifications, when there is only one shown to the user, and only one is expected since all the notifications use a "notifyget" tag.

Comment 1

[Michael Henretty [:mikehenrty][:mhenretty]]

James, do you know if this ever work for you after bug 930794 was fixed? I'm trying to figure out if this was a regression from our recent notification work, or was always a problem.

(In reply to James Burke [:jrburke] from comment #0)
> [snip]
> Furthermore, tapping "Trigger app to close notification" will report closing
> 2 notifications, when there is only one shown to the user, and only one is
> expected since all the notifications use a "notifyget" tag.

Does this mean you see "CLOSING notification with tag: notifyget" twice? If so, this is a separate bug that we should also file.

Comment 2

[James Burke [:jrburke]]

(In reply to Michael Henretty [:mhenretty] from comment #1)
> James, do you know if this ever work for you after bug 930794 was fixed? I'm
> trying to figure out if this was a regression from our recent notification
> work, or was always a problem.

This is the first time I have tried this after bug 930794 was fixed, so cannot speak to whether it is a regression.

> Does this mean you see "CLOSING notification with tag: notifyget" twice? If
> so, this is a separate bug that we should also file.

Yes, it showed twice, which to me implies two notificatiosn with the same tag. I am traveling at the moment, and do not recall if I ever checked the IDs, so not sure if it was the same notification in the get() array listed twice or two different ones.

Comment 3

[Robert Bindar]

I think this bug shows the same problem as Bug 1065535. Let's wait to see that patch landed and then try to reproduce it again.

Comment 4

[Michael Henretty [:mikehenrty][:mhenretty]]

Productivity really wants to fix bug 922722 in 2.2, which depends on this bug. Nom for triage to see if we have bandwidth.

Comment 5

[Evan Tseng [:evanxd]]

I could not reproduce this.

The environment:
Build ID               20141216160204
Gaia Revision          54ae651ebb250a148d4be3bd33db377caaa13c7d
Gaia Date              2014-12-17 06:00:22
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/8401afdb6e6c
Gecko Version          37.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20141110.072548
Firmware Date          Mon Nov 10 07:25:58 EST 2014
Bootloader             L1TC00011880

Comment 6

[Evan Tseng [:evanxd]]

In the step 5 of STR, it showed `NOTIFYGET TOTAL COUNT: 1`.

Comment 7

[Michael Henretty [:mikehenrty][:mhenretty]]

Thanks for checking this Evan!

James could you verify that this is working for your use case in bug 922722?

Comment 8

[James Burke [:jrburke]]

Looks like this is fixed now, test app works as expected as well as upcoming changes for bug 922722 for email. Thanks!

Comment 9

[Evan Tseng [:evanxd]]

Nice. :)