Closed Bug 1052435 Opened 8 years ago Closed 8 years ago
Old SMS notifications reappear after reboot
STR: 1. Send one SMS to a FxOS device: "Foo" 2. Wait for the FxOs device to receive it 3. Send a second SMS to the FxOS device: "Bar" 4. Open the thread in SMS (by opening the SMS app, not taping the notification) 5. Reboot Expected: No notifications appear Actual: The notification for "Foo" appears. It looks like all notifications that have been replaced will replay at reboot.
[Blocking Requested - why for this release]:
Assignee: nobody → lissyx+mozillians
blocking-b2g: --- → 2.0?
Target Milestone: --- → 2.1 S2 (15aug)
Thanks. It looks like there is something bad happening with tags.
STR: 0. Send one SMS to n1 1. Send one SMS to n1 2. Open SMS app Expected: notificationstore.json at step (2) should be empty Actual notificationstore.json at step (2) contains the first sms notification from step (0)
Looks like we were missing some testcases, in fact.
This is actually tested, and the test is green for a nice reason: at http://dxr.mozilla.org/mozilla-central/source/dom/src/notification/test/unit/test_notificationdb.js#158 we create two notifications from the system app, with the same tag and two different IDs. The fun part begins when one notices that the final systemNotification1 and systemNotification2 objects are shareing the same id. This makes the NotificationDB code thinking it's the same notification, hence hiding the bug.
This commit fixes two bugs. When sending two notifications with the same tag, the NotificationDB this.byTag member was not properly updated. This resulted in multiple notifications with the same tag being saved. NotificationStorage's cache may hide this during the running session, but on B2G the resend of notifications at reboot would expose the issue. The second bug fixed is the test that makes sure we properly overwrite notifications with the same tag in the database: the way we handled fake notification object made them sharing the same ID. This, NotificationDB would not even consider the tags and this lead to hiding the bug.
Comment on attachment 8471699 [details] [diff] [review] Fix handling of Notification with tag Michael, do you mind to review this?
Attachment #8471699 - Flags: review?(mhenretty)
Comment on attachment 8471699 [details] [diff] [review] Fix handling of Notification with tag Review of attachment 8471699 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #8471699 - Flags: review?(mhenretty) → review+
Thanks. Try is all green, tree is closed :(
Beatriz, you may want to know about this one.
Malfunctioning new feature for 2.0, blocking+ from your friendly neighborhood triage team.
blocking-b2g: 2.0? → 2.0+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(In reply to Alexandre LISSY :gerard-majax from comment #11) > Beatriz, you may want to know about this one. Thanks Alexandre! Testing in 2.0 Flame today build(Gecko 1dc3556, Gaia 8d051e3) the issue is not reproduced in our side anymore. Sms notifications of read messages are not displayed after rebooting.
Verified the issue is fixed on 2.1 and 2.0 Flame No old notifications appears after rebooting "Flame 2.1 Device: Flame 2.1 (319mb)(Kitkat Base)(Shallow Flash) BuildID: 20141120001207 Gaia: f8d3bf44029e0afc0124600a4bb34dba8fc1ad21 Gecko: f70a67a7f846 Version: 34.0 (2.1) Firmware: V188-1 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0" "Flame 2.0 Device: Flame 2.0 (319mb)(Kitkat Base)(Shallow Flash) BuildID: 20141120000206 Gaia: 1ede2666f1e6c1b3fd3b282011caf0cbc59544b0 Gecko: 54f1b0ee07a6 Version: 32.0 (2.0) Firmware: V188-1 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0"
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
You need to log in before you can comment on or make changes to this bug.