sync-1 Reporter
	Description • 11 years ago

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:

	xiupinglong
	Comment 1 • 11 years ago

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?

	Jan Jongboom [:janjongboom] (inactive)
	Comment 2 • 11 years ago

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

	Jan Jongboom [:janjongboom] (inactive)
	Comment 3 • 11 years ago

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

	Jan Jongboom [:janjongboom] (inactive)
	Comment 4 • 11 years ago

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.

	Joe Cheng [:jcheng] (please needinfo)
	Comment 5 • 11 years ago

triage: tef+ for GCF certification blocker

	Francisco Jordano [:arcturus] [:francisco]
	Comment 6 • 11 years ago

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!

	Fernando Campo (:fcampo)
	Comment 7 • 11 years ago

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 :)

	Jan Jongboom [:janjongboom] (inactive)
	Comment 8 • 11 years ago

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.

	Fernando Campo (:fcampo)
	Comment 9 • 11 years ago

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.

	Jan Jongboom [:janjongboom] (inactive)
	Comment 10 • 11 years ago

Commented in GH.

	Fernando Campo (:fcampo)
	Comment 11 • 11 years ago

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

	Jan Jongboom [:janjongboom] (inactive)
	Comment 12 • 11 years ago

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?

	Fernando Campo (:fcampo)
	Comment 13 • 11 years ago

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

	James Lal [:lightsofapollo]
	Comment 14 • 11 years ago

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

	Cody Roesch [:croesch]
	Comment 15 • 11 years ago

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

	田旻
	Comment 16 • 11 years ago

(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 • 11 years ago

(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.

	Leo
	Comment 18 • 11 years ago

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

	Leo
	Comment 19 • 11 years ago

@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

	Leo
	Comment 20 • 11 years ago

Hi,

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

Please check.

Thanks,
Leo

	Alex Keybl [:akeybl]
	Comment 21 • 11 years ago

(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.

	Joe Cheng [:jcheng] (please needinfo)
	Comment 22 • 11 years ago

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+

	Alex Keybl [:akeybl]
	Comment 23 • 11 years ago

sync-1@bugzilla.tld - does this impact v1.0.1 for you and remain a blocker? Please make this blocking-b2g:tef? if so.

	田旻
	Comment 24 • 11 years ago

(In reply to Alex Keybl [:akeybl] from comment #23)
> sync-1@bugzilla.tld - 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.

	Jan Jongboom [:janjongboom] (inactive)
	Comment 25 • 11 years ago

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.

	Leo
	Comment 26 • 11 years ago

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

	Etienne Segonzac (:etienne) Assignee
	Comment 27 • 11 years ago

(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.

	Etienne Segonzac (:etienne) Assignee
	Comment 28 • 11 years ago

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.

	Leo
	Comment 29 • 11 years ago

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

	Etienne Segonzac (:etienne) Assignee
	Comment 30 • 11 years ago

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!

	Leo
	Comment 31 • 11 years ago

@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!

	Etienne Segonzac (:etienne) Assignee
	Comment 32 • 11 years ago

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.

	Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
	Comment 33 • 11 years ago

(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.

	Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
	Comment 34 • 11 years ago

And this time let me needinfo? smaug for real.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 35 • 11 years ago

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

	田旻
	Comment 36 • 11 years ago

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

	Etienne Segonzac (:etienne) Assignee
	Comment 37 • 11 years ago

(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.

	Leo
	Comment 38 • 11 years ago

(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