Closed Bug 1010867 Opened 6 years ago Closed 6 years ago

[OPENC_1.3][email/UI] Message notification handling is pushing message_reader onto the stack twice

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
2.0 S2 (23may)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: zhang.ruili, Assigned: jrburke)

Details

(Whiteboard: [cert])

Attachments

(6 files)

52.03 KB, application/x-rar
Details
5.20 MB, video/3gpp
Details
5.20 MB, application/octet-stream
Details
5.20 MB, application/octet-stream
Details
1.61 MB, application/octet-stream
Details
46 bytes, text/x-github-pull-request
asuth
: review+
Details | Review
Attached file BUG 17.rar
User Agent: Mozilla/5.0 (Windows NT 5.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/34.0.1847.131 Safari/537.36

Steps to reproduce:

Delete e-mail in any inbox - after press delete the e-mail is shown again - when pressing back the mail is again shown very often.
OS: All → Gonk (Firefox OS)
Hardware: All → ARM
Hi Ruili -

Could you help to get adb log of this issue? Also it would be very helpful if you can also provide to video of this issue as well

Thanks for your help
Flags: needinfo?(zhang.ruili)
Flags: needinfo?(zhang.ruili)
Here is the video. I cut the video into 4 parts, please unzip them.
Hi Andrew, i put the issue video here: https://www.youtube.com/watch?v=wuJZ_Dj7NWo&feature=youtu.be

Looks like indeed there is some problems while deleting a email. Could you help to check if the attached log captured the information you need?

Thanks
Flags: needinfo?(bugmail)
The logs and video were invaluable in this case, thank you.

The deletion appears to be working fine.  The problem is that when the notification is tapped on, we are pushing the message_reader card twice.  When delete is pressed the first time you can see that we animate back to the previous card.  Unfortunately the previous card is a message_reader for the same message.  Message cards don't automatically close themselves even if their message is deleted.

In the video it looks like only a single tap is being generated, so it's not immediately clear if this is an issue of the notification firing multiple times or a logic problem triggered by the single notification.  I know notifications like to generate a close notification in addition to a click notification, maybe that's happening?

:jrburke, any quick thoughts?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(bugmail) → needinfo?(jrburke)
Summary: [OPENC_1.3]When deleting recieved e-mails the mail is not deleted properly. → [OPENC_1.3][email/UI] Message notification handling is pushing message_reader onto the stack twice
When I look at the log, I see this:

05-15 09:44:45.007 I/GeckoDump(  286): XXX FIXME : Got a mozContentEvent: desktop-notification-click
05-15 09:44:45.007 I/GeckoDump(  286): XXX FIXME : Got a mozContentEvent: desktop-notification-click
05-15 09:44:45.157 I/GeckoDump( 1742): LOG: pushCard for type: message_reader
05-15 09:44:45.277 I/GeckoDump( 1742): LOG: pushCard for type: message_reader

Which indicates a double tap on the notification, so I do think we just get two notification click notifications.

For 1.3, the activity pathway has a time guard to event quick duplicate events, but the notification pathway does not. We could avoid the issue by using the same approach there, although I thought 1.3 was pretty closed up and could see how this bug may not make the triage bar. In that case, we should be sure to fix it on master.

When I tried to reproduce locally, I could get a double tap and double message_reader pushCard, but the one of the pushCards would error out with this error:

I/GeckoDump(  600): ERR: Problem handling message type: gotBody TypeError: domNode is null 
I/GeckoDump(  600):  MessageReaderCard.prototype.buildBodyDom@app://email.gaiamobile.org/js/cards/message_reader.js:857
I/GeckoDump(  600): MessageReaderCard.prototype.postInsert/</<@app://email.gaiamobile.org/js/cards/message_reader.js:327
I/GeckoDump(  600): MailAPI.prototype._recv_gotBody@app://email.gaiamobile.org/js/ext/mailapi/main-frame-setup.js:2814

so if I did not hit that error, I could see where I would end up with two message_reader cards.

The fix for bug 969609 will remove all cards on a notification, but only if the current card is not the message_list. However, in this case, with the fast taps, with the second tap, the current card is still the message_list so the cards are not cleared, since we are still in the process of lazy loading the code for the message_reader for the first tap.

The fast tap guard we have in place for the activities pathway would fix the issue if it were also applied to the notifications pathway.
Flags: needinfo?(jrburke)
Hi James -

Although 1.3 is about to close, but since this issue is a cert blocker, we probably still need to fix it in 1.3, is it possible for your to provide a 1.3 patch that applies the tap guard in the activities pathway to the notification pathway as well to fix this issue?

Thanks for your help
Flags: needinfo?(jrburke)
blocking-b2g: --- → 1.3?
Attached file GitHub pull request
Fix that throttles entry triggers for both activities and notifications. Done against the master branch. 

If this lands on master and gets 1.3, should be easy to uplift, but I can prep custom 1.3 pull request if this gets 1.3+.
Attachment #8426402 - Flags: review?(bugmail)
Flags: needinfo?(jrburke)
Comment on attachment 8426402 [details] [review]
GitHub pull request

r=asuth, minor comment/log requests on pull request.
Attachment #8426402 - Flags: review?(bugmail) → review+
Updated the comment, and added a console log in the case the gate is activated.

Fixed in gaia master:
https://github.com/mozilla-b2g/gaia/commit/43d26d9f869d62616b217b8c68b21d02e1a8da5b

from pull request:
https://github.com/mozilla-b2g/gaia/pull/19492
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
I applied the patch in v1.3. It seems the issue not occur. Thank you.
blocking-b2g: 1.3? → 1.3+
Whiteboard: [cert]
Whiteboard: [cert]
Whiteboard: [cert]
You need to log in before you can comment on or make changes to this bug.