More than 1 Notification doesn't close if an alert is used on Notification onclose

VERIFIED FIXED

Status

Firefox OS
Gaia::System
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: caiolima, Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
timdream
: feedback+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130814063812

Steps to reproduce:

1. Open the "new Notification" from UItest 
2. Open more than 1 Notification
3. Click on "Close all notification"




Actual results:

All the |onclose| functions are called, but only one notification is closed. 


Expected results:

All notification should be closed
(Reporter)

Updated

4 years ago
Depends on: 908978
(Reporter)

Comment 1

4 years ago
I've tested it with the patch https://bugzilla.mozilla.org/attachment.cgi?id=796122 applied

Updated

4 years ago
Blocks: 911002
I'd like to know if that happens in 1.1.
Keywords: qawanted
(In reply to Julien Wajsberg [:julienw] from comment #2)
> I'd like to know if that happens in 1.1.

This isn't possible to reproduce on 1.1 - Web Notifications API is enabled on 1.2 or later.
Keywords: qawanted
Jason> I mean, with the notifications we have in 1.1.

If we have several notifications from the same app (eg: several missing calls, several sms), does "closing all notification" close them all ?
Keywords: qawanted

Updated

4 years ago
QA Contact: dsubramanian

Comment 5

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #4)
> If we have several notifications from the same app (eg: several missing
> calls, several sms), does "closing all notification" close them all ?

Hello Julien,

I had 15 notifications from screenshots, missed calls and SMS. Tapping on "Clear All" on the notification bar, clears all the notification. I was not able to repro the bug as per description.

Environmental variables used for testing:
Leo Build ID: 20130830041201
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/2fb19341d934
Gaia: 577ab5105af418efc8ded00798fd3644b3dac674
Platform Version: 18.1
Keywords: qawanted
(Reporter)

Comment 6

4 years ago
The "clear all" from Notification Center doesn't call the "onclose" method of notification. I guess that it's a case of bug https://bugzilla.mozilla.org/show_bug.cgi?id=908978 but to MozNotification.

I guess that the onclose should be called, because the notification is being closed anyway. What do you think?
(Reporter)

Comment 7

4 years ago
Created attachment 798131 [details] [review]
bug-910997.txt

WIP

It's solving partially the problem, but I don't guess that is the best solution.

As I said in PR, I don't understand why the lines below the |window.dispatch(event)| aren't being executed. 

Could anyone give an idea?
Attachment #798131 - Flags: review?(reuben.bmo)
Attachment #798131 - Flags: review?(anygregor)
Comment on attachment 798131 [details] [review]
bug-910997.txt

Sorry I took this long to reply, this request somehow fell through the cracks of my bugmail. I'm really not qualified to review this code, though, so I'm forwarding this to a Gaia System app peer.
Attachment #798131 - Flags: review?(reuben.bmo) → feedback?(timdream)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 798131 [details] [review]
bug-910997.txt

It looks like you have isolated the error but you didn't provide a proper fix.

Please look for error messages to see if why the function stop executing after that line. Thanks.
Attachment #798131 - Flags: review?(anygregor)
Attachment #798131 - Flags: feedback?(timdream)
Attachment #798131 - Flags: feedback+
Gregor, would it be possible if some thing in shell.js throw and break the code here?
Flags: needinfo?(anygregor)

Updated

4 years ago
Whiteboard: [systemfe]

Updated

4 years ago
Whiteboard: [systemfe] → [systemsfe]
(Reporter)

Comment 11

4 years ago
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #10)
> Gregor, would it be possible if some thing in shell.js throw and break the
> code here?

I've tested the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=910915 It is working well. Jason, could you confirm it to me? 

Confirming it, this bug is closed then!
Marking qawanted for a retest with landing of bug 910915.
Keywords: qawanted
QA Contact: dsubramanian

Updated

4 years ago
blocking-b2g: --- → koi?

Updated

4 years ago
QA Contact: nkot

Comment 13

4 years ago
have tested on the latest, 9/12, Buri build - the issue does not reproduce, tapping "Clear all" clears off all notifications (tested with 10 notifications: SMS ones, missed calls and calendar reminders)

Build ID: 20130912040201
Gecko: http://hg.mozilla.org/mozilla-central/rev/a98569f21abe
Gaia: 9ffd2899eb91388f7fc1ce6f7a895a6f5f922c05
Platform Version: 26.0a1
Keywords: qawanted
Closing per comment 13
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Status: RESOLVED → VERIFIED

Updated

4 years ago
blocking-b2g: koi? → koi+
Flags: needinfo?(anygregor)
(Assignee)

Updated

4 years ago
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.