858908 - [Buri][SMS]If received more than 2 SMS of Class 0 type, just can read one SMS detail infomation

Status: RESOLVED FIXED

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

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.19.053
 Firefox os  v1.0.1
 Mozilla build ID: 20130327153857
 
 +++ This bug was initially created as a clone of Bug #431424 +++
 
 [WuYing][6915]
 
 DEFECT DESCRIPTION:
 If received more than 2 SMS of Class 0 type, just can read one SMS detail infomation
 
 
 REPRODUCING PROCEDURES:
 1.after received more than 2 SMS of Class 0 type
 2.These SMSM  will prompt in notification.
 3.pull dowm the status bar
 4.Select one Class0 SMS to tap
 5.It will show detail information
 6.But other Class0 SMS will disappear,cannot read detail meessage---KO
 
 EXPECTED BEHAVIOUR:
 read all Class0 SMS detail infomation
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 medium
 REPRODUCING RATE:
 5/5
 For FT PR, Please list reference mobile's behavior:
 
 ++++++++++ end of initial bug #431424 description ++++++++++
 
 
 
 CONTACT INFO (Name,Phone number):
 
  DEFECT DESCRIPTION:
 
  REPRODUCING PROCEDURES:
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[xiupinglong]

According the currently desktop-notification, while app launch the notifications belong it will disappear. It causes more than two class0 messages are not be read the details. Who can deal with it?

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

Identified as real problem, based on the triage meeting of this morning upping to tef+.

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

Oh I can't do that, then this is a message for the triage meeting for tomorrow :-)

Comment 4

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch

The problem lies herein:

Class 0 SMSes are handled by the SMS app. When the SMS app encounters a class 0 sms it shows a notification.
When clicking a notification the respective application will launch
All notifications for this application will be removed
If you had two class 0 SMSes, or one in notifications that you didn't read yet, it will be purged, because these SMSes don't show up in the SMS app (on purpose).
This patch introduces a way for notifications to avoid getting cleared when opening their parent application. The behaviour when clicking the notification (goes away) or 'clear all' is not affected.

So I took maybe a bit of an unusual approach, but that's because I didn't want to touch Gecko on this part (and change the API of mozNotification), probably mainly because this is a FFOS only problem so far. If you prefix the title line of the notification with a zero width space it will become a 'persistent' notification.

The reason for this is that it doesn't matter if a bug is introduced here, because the character will never be shown. In theory it could cause problems if you create people in your contacts that have a name that starts with a zero width space, but a la.

Comment 5

[Joe Cheng [:jcheng] (please needinfo)]

triage: tef+ for GCF certification blocker

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 737961 [details] [diff] [review]
Patch

Fernando could you take a look to this r?

Thanks Jan, just left some feedback on the PR in github.

Great job!

Comment 7

[Fernando Campo (:fcampo)]

Comment on attachment 737961 [details] [diff] [review]
Patch

Yup, taking a first look, I don't think that modifying notifications is a good idea for a sms-specific situation. 

We should change the way 0-type sms are read instead, e.g. not deleting the sms until the alert is shown.

We can chat about it face to face to speed it up if you want :)

Comment 8

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 737961 [details] [diff] [review]
Patch

Changed the behavior, now fires up SMS app and shows alert immediately. Note: if you're in the lock screen and a class0 comes in you won't see it until you unlock the phone.

Comment 9

[Fernando Campo (:fcampo)]

I like this fix, simpler is better IMO, and the lock screen issue is not that bad as the user is gonna see the message as soon as he unlocks the phone. Just a comment on github.

Comment 10

[Jan Jongboom [:janjongboom] (inactive)]

Commented in GH.

Comment 11

[Fernando Campo (:fcampo)]

Comment on attachment 737961 [details] [diff] [review]
Patch

Cool, looks great, thanks for the quick fix, and sorry for the slow review :(

Bonus item: tt also solves the bug 846293, as it gets rid of the notifications

Comment 12

[Jan Jongboom [:janjongboom] (inactive)]

Landed in https://github.com/mozilla-b2g/gaia/commit/444b622a5c19229517a9a813ad4d6711bb130382

Do I need to do something special because this has to go in 1.0.1 too?

Comment 13

[Fernando Campo (:fcampo)]

Yup, just change the flag so uplifters get notice (I just did).

Comment 14

[James Lal [:lightsofapollo]]

Uplifted 444b622a5c19229517a9a813ad4d6711bb130382 to:
v1-train: c2e5f6c7d5017e48de8845e986d4b2d1795f6be9
v1.0.1: 6cb6aabd667ec92a542ead1f5f96e855ae9da771

Comment 15

[Cody Roesch [:croesch]]

Unable to test with a Buri Device. Marked as QARegressExclude.

Comment 16

[田旻]

(In reply to croesch from comment #15)
> Unable to test with a Buri Device. Marked as QARegressExclude.

Test on FF OS phone of  
AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.094
Firefox os  v1.0.1
Mozilla build ID:20130502070201.

Problems still exist.

Comment 17

[田旻]

(In reply to croesch from comment #15)
> Unable to test with a Buri Device. Marked as QARegressExclude.

I don't know why your test of this bug is ok. But If I send several class-0 messages and waited enough time when all the class-0 message arrived at the ME, only the last one could be seen. Others were flashed out by the last one.

Comment 18

[Leo]

Tested this defect but the defect still exist.

Reproduced Steps:
1.Receive two messages of class-0 type.
2.The alert is displayed for the same.
3.Click on Ok button in the alert message.
4.When user clicks on alert message the 2 messages are cleared.
Expected:The second alert message should not be cleared when the Ok on the first alert message is clicked.

Please check once.

Thanks,
Leo

Comment 19

[Leo]

@Jan Jongboom [:janjongboom] 

Please check the issue once again.
Able to reproduce this defect.

STR Steps:
1.Receive two messages of class-0 type.
2.The alert is displayed for the same.
3.Click on Ok button in the alert message.
4.When user clicks on alert message of the first message the 2 message alert also cleared.
Expected:The second alert message should not be cleared when the Ok on the first alert message is clicked.

Thanks,
Leo

Comment 20

[Leo]

Hi,

Reopening this issue.
As this fix is not working in master and in v1.

Please check.

Thanks,
Leo

Comment 21

[Alex Keybl [:akeybl]]

(sorry to steal from you janjongboom, we just have very near deadlines and I don't know your availability :)

Etienne - can you take a look? If not, maybe fernando.campo would be a good assignee.

Comment 22

[Joe Cheng [:jcheng] (please needinfo)]

can we confirm if v1.0.1 is impacted?
this is reopened for master and v1 per comment 20
if only for master and v1, this should be a leo+, instead of reopening a tef+

Comment 23

[Alex Keybl [:akeybl]]

[e-mail] - does this impact v1.0.1 for you and remain a blocker? Please make this blocking-b2g:tef? if so.

Comment 24

[田旻]

(In reply to Alex Keybl [:akeybl] from comment #23)
> [e-mail] - does this impact v1.0.1 for you and remain a blocker?
> Please make this blocking-b2g:tef? if so.

I can't definitely sure whether it is a blocker and impact v1.0.1. May be it's not a blocker. But it is the basic function in the test case.

Comment 25

[Jan Jongboom [:janjongboom] (inactive)]

Feel free to steal :-). Anyway, if this is a bug than I guess it's a Gecko bug related to alert messages. Can anyone test:

alert(1);alert(2); 

should display two alert boxes.

Comment 26

[Leo]

Hi Jan Jongboom,

As per my analysis this bug on alert messages need to handled in System apps in modal_dialog.js
'mozbrowsershowmodalprompt' event is getting called when we receive an alert message.
When we click on ok button of an alert message confirmHandler() function is called where we need to handle this case.

Thanks,
Leo

Comment 27

[Etienne Segonzac (:etienne)]

(In reply to Jan Jongboom [:janjongboom] from comment #25)
> Feel free to steal :-). Anyway, if this is a bug than I guess it's a Gecko
> bug related to alert messages. Can anyone test:
> 
> alert(1);alert(2); 
> 
> should display two alert boxes.

This case works, because we won't execute |alert(2)| until you clicked ok on the first alert.

It get's trickier here since the the app can actually receive the second system-message (and execute the second call to alert()) while the first alert is being displayed.

Anyway, I can reproduce the bug, I'll try something.

Comment 28

[Etienne Segonzac (:etienne)]

The issue is being tracked on bug 874892.

I opened a new bug since it's now a system app issue and not really a follow up to this first patch.

Closing this bug.

Comment 29

[Leo]

Hi,

After sending two class-0 messages send one normal SMS,then the the normal SMS is showing in notification but the same thing is not updating in the thread_list.Once if we restart the device then only we can able to see the normal message in the thread_list.

Please check the scenario once.
Reproduced on v1-train.

Thanks,
Leo

Comment 30

[Etienne Segonzac (:etienne)]

Vivien, my understanding of how the JS execution is stopped/resumed isn't sharp enough.
Do you have any pointer on what we could do concerning Comment 29?

Thanks!

Comment 31

[Leo]

@Etienne 

Reproduced this in 2 cases
1.Receive 2 class-0 messages for the first time,receive one normail sms.The normal SMS is not updating in thread_list.
2.After receiving class-0 messages,try sending sms from existing number.When we come back the thread_list is empty.Once restart then the device shows the existing messages

(In reply to Etienne Segonzac (:etienne) from comment #30)
> Vivien, my understanding of how the JS execution is stopped/resumed isn't
> sharp enough.
> Do you have any pointer on what we could do concerning Comment 29?
> 
> Thanks!

Comment 32

[Etienne Segonzac (:etienne)]

Looked into in a bit more:

- if the app is already launched, we receive the system message and the mozMobileMessage received event

- if the app is launched by the system message, we don't get the mozMobileMesage received event, which is used to update the UI.

So we get the app in a weird state and have to restart it in order to see the message.

Comment 33

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Etienne Segonzac (:etienne) from comment #32)
> Looked into in a bit more:
> 
> - if the app is already launched, we receive the system message and the
> mozMobileMessage received event
> 
> - if the app is launched by the system message, we don't get the
> mozMobileMesage received event, which is used to update the UI.
> 
> So we get the app in a weird state and have to restart it in order to see
> the message.

We discussed with Etienne IRL today. So the main issue here is that when an application enter a modal state the event handling is turned off afaict. (Let's needinfo smaug to make sure). 

Smaug, when an application makes a call to window.alert in b2g we call nsIDOMWindowUtils.enterModalStateWithWindow (at http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#293) with I think disable event handling at http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#7173 ?

Is that correct?


If yes, I would suggest to use only the system message to react on new sms/mms in order to update the list. That should resolve the bug and  make the code more consistent.

Comment 34

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

And this time let me needinfo? smaug for real.

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

So events from the user aren't dispatched to the window when the window is in modal state.

Comment 36

[田旻]

Thus maybe we should reopen this bug for all the above problems, or at least fire a new bug for this serious of problems.

Comment 37

[Etienne Segonzac (:etienne)]

(In reply to 田旻 from comment #36)
> Thus maybe we should reopen this bug for all the above problems, or at least
> fire a new bug for this serious of problems.

Since the description and STR are different I think a new bug would make sens.

Comment 38

[Leo]

(In reply to Etienne Segonzac (:etienne) from comment #37)
> (In reply to 田旻 from comment #36)
> > Thus maybe we should reopen this bug for all the above problems, or at least
> > fire a new bug for this serious of problems.
> 
> Since the description and STR are different I think a new bug would make
> sens.

I have filed a new bug.
Bugzilla ID: 876614.Please check it