796904 - [sms] messages of class 0 shows in status bar & lock screen but not available in messages view

Status: RESOLVED FIXED

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

Description

[Vicamo Yang [:vicamo][:vyang]]

When underyling ril component has received a message of message class 0, by specification, it skips message saving and notifies content directly. User sees a new message shown in both status bar and lock screen. But when he goes to message app and try to read the received message, there is nothing found, because that message was not saved in database.

Here is an use case in SIM activation process. The carrier sends a SMS of message class 0 to notify the user of service unavailable or similar events:

> I/Gecko   (  106): -*- RadioInterfaceLayer: Received message from worker:
> {"SMSC":"[phone]","mti":0,"udhi":0,"sender":"+8955","recipient":null,
> "pid":0,"epid":0,"dcs":240,"mwi":null,"replace":false,"header":null,"data":null,
> "timestamp":1349169612000,"status":null,"scts":null,"dt":null,"encoding":0,
> "messageClass":0,
> "fullBody":"Por favor, aguarde alguns minutos, pois a ativacao esta em andamento",
> "rilMessageType":"sms-received"}

See also bug 736706, 3GPP TS 23.038, 3GPP TS 23.040.

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

The `message class` attribute is not exposed to content through the SMS API now. We'll need that to perform extra handling for class 0 messages.

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Attachment: screenshot

attached is the ss of a Android device showing a just received class 0 message.

Comment 3

[Beatriz Rodríguez [:brg]]

Attachment: Class 0 message

Comment 4

[Beatriz Rodríguez [:brg]]

Attachment: after selecting save option, the sms is in the inbox

Comment 5

[Beatriz Rodríguez [:brg]]

if user press cancel, the sms is not stored

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #1)
> The `message class` attribute is not exposed to content through the SMS API
> now. We'll need that to perform extra handling for class 0 messages.

Either this is a DOM API bug, or this is a Gaia bug blocked by a DOM API bug.

Is that correct? Please file or change the product/component accordingly.

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #6)
> (In reply to Vicamo Yang [:vicamo][:vyang] from comment #1)
> > The `message class` attribute is not exposed to content through the SMS API
> > now. We'll need that to perform extra handling for class 0 messages.
> 
> Either this is a DOM API bug, or this is a Gaia bug blocked by a DOM API bug.
> 
> Is that correct? Please file or change the product/component accordingly.

Ah, filed a DOM API bug 797277.

And, there is still one more thing to discuss about. The UI attached by Beatriz supports saving the message after user's confirmation. This is also impossible for now because that message has either already been saved or will be discarded after the notification. I guess we'll need something like asking users for permission, but I'm afraid that it will show another popup and confuse users.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Vicamo, I thought the desired behavior (for Brazil) is simply put the received message on the line 2 of the carrier info on lock screen, as https://github.com/mozilla-b2g/gaia/issues/4559 suggests?

(Gods I hate this Github - Bugzilla separation)

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #8)
> Vicamo, I thought the desired behavior (for Brazil) is simply put the
> received message on the line 2 of the carrier info on lock screen, as
> https://github.com/mozilla-b2g/gaia/issues/4559 suggests?
> 
> (Gods I hate this Github - Bugzilla separation)

No, that's different things. For v1, I'd rather have a popup that contains only a OK button, that is, the message can't be saved and you'll need a message class attribute to know which message should bring up a popup. What do you think?

Comment 10

[Dietrich Ayala (:dietrich)]

Waiting for reply from Daniel for clarification on whether this blocks, and a specification about what class 0 is.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Dietrich Ayala (:dietrich) from comment #10)
> Waiting for reply from Daniel for clarification on whether this blocks, and
> a specification about what class 0 is.

For 2), please see bug 797277 comment #2. In short, that's a tag means the message should not be stored by default, but UI can still provide other options. Because we don't have this attribute exposed in API, then there is no chance that UI can provide alternative options at all.

Comment 12

[Mounir Lamouri (:mounir)]

How bad would that be to simply consider those messages as regular messages? (ie. saved in the DB)

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Mounir Lamouri (:mounir) from comment #12)
> How bad would that be to simply consider those messages as regular messages?
> (ie. saved in the DB)

That's not specification conformal. Shouldn't we follow the standard as possible?

Comment 14

[Mounir Lamouri (:mounir)]

It's not really for me to say. However, we are past feature freeze I would say that at that point, we should just try to find a compromise.

Comment 15

[Beatriz Rodríguez [:brg]]

This is a basic functionality in SMS behaviour. IMHO we should follow specs because there is a GCF (Global certification forum) (similar to PTCRB but in Europe) requirement. In GCF all the test cases are mandatory and it is important to get that certification. The test case is: SMS-1.3.1 (GCF 2G & 3G)

We cannot measure the impact of not supporting this basic functionality in all the countries. Many different operator services use them for different reasons.

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

Chris - Can you give some feedback on this? Can these messages be treated as normal messages?

Comment 17

[Dietrich Ayala (:dietrich)]

Pre-emptive blocking because required for certification.

Vicamo, what's required to fix this bug? Assigning to you for now for estimation. We can re-assign once you explain what's needed to fix.

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Dietrich Ayala (:dietrich) from comment #17)
> Vicamo, what's required to fix this bug? Assigning to you for now for
> estimation. We can re-assign once you explain what's needed to fix.

I've opened bug 797277 and have had some WIP patches. Bug 797277 adds new SmsMessage attribute `messageClass` so Gaia can identify a message categorized as Message Class 0 and do what ever it wants to do. In comment #9, for V1, I suggested we have a dialog showing the text message with only one button "OK" and still discard the message without saving it.

Comment 19

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Mounir Lamouri (:mounir) from comment #12)
> How bad would that be to simply consider those messages as regular messages?
> (ie. saved in the DB)

Just got SMS test cases from one of our partners. One of them is for message class 0 and is marked as critical. The test steps are:

1. send a SMS with message class set to 0 via test tool to device under test(DUT).
2. ensure the message is displayed but not stored on DUT.
3. turn off DUT and turn on again.
4. ensure the message is still not stored on DUT.

Comment 20

[Mounir Lamouri (:mounir)]

I was reading about those class 0 messages and I found something interesting [1]. If someone send you a class 0 message that you can't store in the phone's memory and that doesn't show the number of the sender, this can be used to spoof the user because the user might thing this is a system popup or even a carrier information. Seems like the iPhone wasn't allowing the user to save class 0 messages and some users were receiving message like "Virus detected, you should restore your phone.". No big issue but better to not fall to learn from other's mistakes.

[1] (fr) http://www.journaldugeek.com/2012/06/22/un-mysterieux-virus-sevit-sur-iphone/

Comment 21

[Beatriz Rodríguez [:brg]]

(In reply to Mounir Lamouri (:mounir) from comment #20)
> No big issue but better to
> not fall to learn from other's mistakes.
> 
Agree! It is easy if we just follow the specs! and do not store them in the device.

Comment 22

[Alex Keybl [:akeybl]]

We're marking this bug with the C1 milestone since it follows the criteria of "unfinished feature work" (see https://etherpad.mozilla.org/b2g-convergence-schedule).

If this work is not finished by Nov19, this bug will need an exception and will be called out at the upcoming Exec Review.

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Dietrich Ayala (:dietrich) from comment #17)
> Vicamo, what's required to fix this bug? Assigning to you for now for
> estimation. We can re-assign once you explain what's needed to fix.

Please help find one Gaia developer for this. I've explained what to do in comment #18.

Comment 24

[Borja Salguero [:borjasalguero]]

We need to make a workaround because background service is not working with prompts.... and one solution could be use 'system-messages'. I would update the target milestone for next one, due to following week I will create the patch with [:etienne]

Comment 25

[Alex Keybl [:akeybl]]

Certification requirement - granting an exception here and moving into the C2 milestone.

Comment 26

[Dietrich Ayala (:dietrich)]

Any update on the approach here?

Comment 27

[Borja Salguero [:borjasalguero]]

Tomorrow I will test the approach and if everything works as expected I will make the PR. It's a little bit complicated to test because not all carries have this type of message (or it's not easy to generate).

Comment 28

[Borja Salguero [:borjasalguero]]

Attachment: PR

Comment 29

[Borja Salguero [:borjasalguero]]

Tested with [:brg] and it's working as expected ;).

Comment 30

[Etienne Segonzac (:etienne)]

My comments are on github, should be easy enough to fix! :)

Comment 31

[Borja Salguero [:borjasalguero]]

All comments addressed!

Comment 32

[Etienne Segonzac (:etienne)]

Can you do those last changes so we can merge? (last comment + commit message)

Comment 33

[Borja Salguero [:borjasalguero]]

[:etienne] Sorry for the delay, it's complicated to test! All suggestions added. Thanks!

Comment 34

[Etienne Segonzac (:etienne)]

Comment on attachment 688174 [details]
PR

Thanks!

Comment 35

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/8f3578363db5ad9aaf25d7a86335fedbed6519c8