Closed Bug 979462 Opened 11 years ago Closed 10 years ago

SMS notifications don't clear after being selected

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::SMS
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 997949

People

(Reporter: benfrancis, Unassigned)

References

Details

(Keywords: regression) 

Pre-requisites:
* Have received an SMS but not viewed it yet.

STR:
* Open utility tray
* Tap on notification
* View SMS
* Open utility tray

Expected:
* Notification has been removed

Actual:
* Notification still there, have to manually swipe it away to remove it.

Tested on a Keon running a recent master build.

Sorry if this is a dupe, I couldn't find it filed.
This seems to apply to missed call notifications as well, so possibly all notifications.
Let's verify if this reproduces on Buri.
Keywords: regressionwindow-wanted → qawanted
Component: Gaia::System → Gaia::SMS
I'm quite sure Julien knows this, but FYI, if SMS is using the new Notification API they will have to manually close their notifications when receiving the click event, otherwise the notification will stick around in the system tray.
Was unable to reproduce this on latest 1.4 build on Buri

Result:
Clicking notification in tray opens Messages app to new SMS thread; Reopening notification tray shows SMS notification is no longer present.

Environmental Variables:
Device: Buri v1.4 Master Mozilla RIL
BuildID: 20140306040204
Gaia: 9cb35e701df44766d9b3560b0defe0a401a0ecdd
Gecko: 8122ffa9e1aa
Version: 30.0a1
Firmware Version: v1.2-device.cfg
Keywords: qawanted
Clearing nom as this does not reproduce on a production device.
blocking-b2g: 1.4? → ---
(In reply to Michael Henretty [:mhenretty] from comment #3)
> I'm quite sure Julien knows this, but FYI, if SMS is using the new
> Notification API they will have to manually close their notifications when
> receiving the click event, otherwise the notification will stick around in
> the system tray.

Yep, Alexandre Lissy himself did this in https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/message_manager.js#L346-L358

A user also filed bug 977533 which is probably a dupe. This means 2 separate and trustable users have the issue. serious enough in my opinion.

Ben, can you do a video of the whole steps?
See Also: → 977533
Just want to mention that I tried it myself from the latest 1.3 build from Geeksphone and I don't reproduce.

So we definitely need a video, Ben, if you can make one.
Flags: needinfo?(bfrancis)
Found a case where the notification is not cleared: see bug 977533 comment 9.
Happened on my Fugu and Buri(twice, but hard to reproduce)

Fugu http://youtu.be/J3ZyPLozB0o
Gaia      d2651c13d6370d62bf833b31c7e2e5f054510136
Gecko     c8a17e25111585c0eebb829f8208190ea432c71e
BuildID   20140318053055
Version   30.0a1
Eric, the first one could be bug 981401.

The second one, I don't know. It looks like we can't find the correct notification to close it...

We really need a good STR here :/
Not able to reproduce it since then... keep trying.
Ben, Eric, do you remember if the SMS thread looked fine after you select a notification? For example, did you see bug 988726?
If this applies to all notifications then it might be a regression of bug 991666 which I landed to fix an issue with old-style notifications. AFAIK that's the most recent change to the shared notification code; I'll have a look with and without that patch applied to see if the problem goes away.
I've quickly tried reverting bug 991666 but it doesn't fix the problem so the issue must be somewhere else. One thing worth noting is that tapping on the notification toaster removes the notification correctly, it's only when tapping on the utility tray that causes the notification not to be removed.
(In reply to Gabriele Svelto [:gsvelto] from comment #16)
> I've quickly tried reverting bug 991666 but it doesn't fix the problem so
> the issue must be somewhere else. One thing worth noting is that tapping on
> the notification toaster removes the notification correctly, it's only when
> tapping on the utility tray that causes the notification not to be removed.

Gabriele, you did reproduce the issue?

The fact that tapping the toaster works sounds very weird to me. I wonder if we're doing the right thing when tapping the toaster...
(In reply to Julien Wajsberg [:julienw] from comment #17)
> Gabriele, you did reproduce the issue?

Yes, I could reproduce it both on my Peak with GP's nightly build and on my Buri using my own master build

> The fact that tapping the toaster works sounds very weird to me. I wonder if
> we're doing the right thing when tapping the toaster...

Disregard my previous message, I just tried again and it fails when tapping the toaster too. I've added a couple of quick dumps into the code and it seems that the event that is delivered does not contain the tapped node for some reason: the target field holds an empty object.
(In reply to Gabriele Svelto [:gsvelto] from comment #18)
> (In reply to Julien Wajsberg [:julienw] from comment #17)
> > Gabriele, you did reproduce the issue?
> 
> Yes, I could reproduce it both on my Peak with GP's nightly build and on my
> Buri using my own master build
> 
> > The fact that tapping the toaster works sounds very weird to me. I wonder if
> > we're doing the right thing when tapping the toaster...
> 
> Disregard my previous message, I just tried again and it fails when tapping
> the toaster too. I've added a couple of quick dumps into the code and it
> seems that the event that is delivered does not contain the tapped node for
> some reason: the target field holds an empty object.

Are you sure it's not because it's not enumerable and so console.log shows an empty object?

When I did investigate, the SMS app got the event but the Notification.get Promise was never resolved nor rejected. See bug 997949, I'm pretty sure it's the same.

Also, do you see bug 988726? (header for the thread you click, but messages from the thread you were)
(In reply to Julien Wajsberg [:julienw] from comment #19)
> Are you sure it's not because it's not enumerable and so console.log shows
> an empty object?

No, you're right, it could be. I was just JSON.stringify()ing the node so if it's not enumerable it wouldn't be dumped out.

> When I did investigate, the SMS app got the event but the Notification.get
> Promise was never resolved nor rejected. See bug 997949, I'm pretty sure
> it's the same.

Could be; BTW I'm not sure I can reproduce this anymore, I've updated both my Peak and Buri this morning and on both the notifications for incoming SMSes would go away when clicked.

> Also, do you see bug 988726? (header for the thread you click, but messages
> from the thread you were)

No, that's not happening.
(In reply to Gabriele Svelto [:gsvelto] from comment #20)
> Could be; BTW I'm not sure I can reproduce this anymore, I've updated both
> my Peak and Buri this morning and on both the notifications for incoming
> SMSes would go away when clicked.

because IMO it happens when the phone is used for quite some time. So if you reboot the phone you don't see it until it happens again.
(In reply to Julien Wajsberg [:julienw] from comment #21)
> because IMO it happens when the phone is used for quite some time. So if you
> reboot the phone you don't see it until it happens again.

My Peak is on the nightly channel so it gets updated every two or three days at most, but usually daily. I'll try to leave it behind for a week and see if the problem crops up again.
See Also: → 997949
ok, cleaning up and duping to the bug with the most information.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Flags: needinfo?(bfrancis)
You need to log in before you can comment on or make changes to this bug.