1062034 - [Messages] [1.4] Upon receiving empty message, notification containing "(null)" is displayed

Status: RESOLVED FIXED

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

Description

[nulti-korisnik]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Firefox/31.0
Build ID: 20140716183446

Steps to reproduce:

This was produced on GP Keon using Gecko-ec1a4d3; Gaia-d6a1e93.

How to produce:
- recieve any empty SMS message;

There are other bugs that are addressing this issue in other apps too:
- Email: https://bugzilla.mozilla.org/show_bug.cgi?id=1057063
- Dialer: https://bugzilla.mozilla.org/show_bug.cgi?id=1039850

These messages should be much more user firendly.


Actual results:

Upon receiving empty message, notification containing "(null)" is displayed


Expected results:

Upon receiving empty message, notification should be something as "(empty message)" or anything that will look normal to regular users.

Comment 1

[Julien Wajsberg [:julienw]]

qawanted for branch checks on Flame.

Thanks !

Comment 2

[Duane Dixon [:ddixon]]

Branch Check

Unable to reproduce issue in Flame 2.2, 2.1, 2.0, 1.4 in v123 and v165 base images (KK base)

Actual Results: Blank MMS appears correctly as a Notification. 

Flame v123 base image

Device: Flame Master
BuildID: 20140912040204
Gaia: 6cb5e0100d70313e4922c8d34bf20dcdd66ef616
Gecko: 2db5b64f6d49
Version: 35.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
------------------------------------------------------------------------
Device: Flame 2.0
BuildID: 20140915094735
Gaia: 7edd3b0b9f65c3dde235c732d270e43e055a1254
Gecko: bb6a6337270c
Version: 32.0 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
------------------------------------------------------------------------
Device: Flame 2.1
BuildID: 20140915120232
Gaia: 66af962728700c6d2cc13e93f8ed5d122305185a
Gecko: 9dce8a7d62fc
Version: 34.0a2 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
------------------------------------------------------------------------
Device: Flame 1.4
BuildID: 20140910070823
Gaia: 6018a1c18f0c3eab25aac2ba3064904740591dd2
Gecko: 39c9dbc311f7
Version: 30.0 (1.4) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
------------------------------------------------------------------------
------------------------------------------------------------------------
Flame KK base image

Device: Flame Master
BuildID: 20140915091417
Gaia: 855be6ade407c26e0596e7306a44deebc3f60933
Gecko: d08c31df0c1a
Version: 35.0a1 (Master) 
Firmware Version: 
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
------------------------------------------------------------------------
Device: Flame 2.1
BuildID: 20140915120232
Gaia: 66af962728700c6d2cc13e93f8ed5d122305185a
Gecko: 9dce8a7d62fc
Version: 34.0a2 (Master) 
Firmware Version: 
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
------------------------------------------------------------------------
Device: Flame 2.0
BuildID: 20140915135336
Gaia: 7edd3b0b9f65c3dde235c732d270e43e055a1254
Gecko: f3639e825b3b
Version: 32.0 (2.0) 
Firmware Version: 
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
------------------------------------------------------------------------
Device: Flame 1.4
Build ID: 20140814202332
Gaia: 129211661489feb60bbd6772a44081d23b374f17
Gecko: 1483bd406aad0b3b9e3bdb75c5238b787b5a0cd6
Version: 30.0 (1.4)
Firmware Version: 
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 3

[Julien Wajsberg [:julienw]]

QA: the bug is about receiving an empty SMS, not an empty MMS.

I think Firefox OS can't send an empty SMS so you'll need to use another phone.

Thanks !

Comment 4

[Duane Dixon [:ddixon]]

Please excuse my typo, above.  I sent the phone SMS's, not MMS's. I performed the branch checks with the following STR...

STR: 
1. Install Flame KK base image and build. 
2. Send an empty MMS to phone from an email address.  
3. Phone receives empty MMS.

Actual Results: 
Phone receives empty SMS correctly; notification does not say "null". 

Note: Flame will NOT send an empty SMS which is why I used an email account to send the message.

Comment 5

[Julien Wajsberg [:julienw]]

Duane, when you send a message by email, I think we really receive a MMS, not a SMS. You can check this if you longpress the message and view the message report.

I'd like that you use a separate phone, if possible, to send this empty SMS. Thanks!

Comment 6

[Cody Roesch [:croesch]]

Looks like we don't have a device here that can send and empty sms. The send button is always grayed out until something is inputed into the text field. We can send just a space but that won't cause the bug. We don't have Keon devices here. We have iphones, androids and phones running FX OS.

Comment 7

[Julien Wajsberg [:julienw]]

I wonder if feature-phones can do this... since they're more usual in our target countries.

Removing the QAwanted.

Bevis, do you know how Gecko reacts to empty messages? Do we get a "null" as body? I wonder if we should have "" as body instead, or if we should handle this in Gaia.

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

This can be reproduced with the emulator by sending an SMS PDU with empty body:
For example,
"sms pdu 0791889613470423040C9188966536996900086070425172342300"

In current gecko implementation, the body is expected to be null if there is no text available.
We could make this change for MOZ_RIL.
However, we are not able to force other vendor's implementation to have "" in this case. :(

Comment 9

[Julien Wajsberg [:julienw]]

Thanks, I don't mind, we can fix it on our side.

That said, I think I recall having fixed this already. Will dig tomorrow.

Comment 10

[Julien Wajsberg [:julienw]]

Ok, we handle it correctly in thread_ui.js and thread_list_ui.js, but not in activity_handler.js

Putting this bug in the mentored bugs list :)

The fix should be to replace "message.body" by "message.body || ''" when calling "continueWithNotification", or do the same thing when defining the options inside "continueWithNotification".

Of course a fix should come with unit tests :)

Comment 11

[kumar rishav (:rishav_)]

Attachment: PR

Hi Juliew
Here is the patch. Have a look and Hope everything should be ok. Please let me know if i missed anything. Treeherder is green too. 
Thanks.

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 8505244 [details] [review]
PR

The unit test is incorrect. Remember a good unit test is a test that fails without your change, and passes with your change.

Thanks !

Comment 14

[kumar rishav (:rishav_)]

Comment on attachment 8505244 [details] [review]
PR

Hi
Have a look on PR.
Thanks

Comment 15

[Julien Wajsberg [:julienw]]

Comment on attachment 8505244 [details] [review]
PR

r=me but please fix the 2 nits before asking checkin-needed :)

Comment 16

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/dce6ee37d6636c094bf263ab7c0af4e1d9d1adf9