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

Status: RESOLVED FIXED

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

Description

[RuiLi]

Attachment: 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.

Comment 1

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

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

Comment 2

[RuiLi]

Attachment: VID_20140519_102706.3gp.001

Here is the video. I cut the video into 4 parts, please unzip them.

Comment 3

[RuiLi]

Attachment: VID_20140519_102706.3gp.002

Comment 4

[RuiLi]

Attachment: VID_20140519_102706.3gp.003

Comment 5

[RuiLi]

Attachment: VID_20140519_102706.3gp.004

Comment 6

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

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

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

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?

Comment 8

[James Burke [:jrburke]]

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.

Comment 9

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

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

Comment 10

[James Burke [:jrburke]]

Attachment: 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+.

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8426402 [details] [review]
GitHub pull request

r=asuth, minor comment/log requests on pull request.

Comment 12

[James Burke [:jrburke]]

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

Comment 13

[RuiLi]

I applied the patch in v1.3. It seems the issue not occur. Thank you.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/744a528172b5ed7b2237690adfaeeb734c1aead9
v1.3: https://github.com/mozilla-b2g/gaia/commit/5bd226b03a2d63dfe9df204f7c0afb9984e8fd42