978553 - [Messages] "TypeError: item is null" when starting the "send sms" activty from a Contact card

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

STR:
1. launch Messages app
2. press home
3. launch the Contacts app
4: display a contact
5. press the "send SMS" button

We get this log:

02-28 19:55:16.235  1567  1567 E GeckoConsole: [JavaScript Error: "TypeError: item is null" {file: "app://sms.gaiamobile.org/js/compose.js" line: 165}]


I'm not sure the first and second steps are necessary to reproduce the issue.

Comment 1

[Gabriele Svelto [:gsvelto] (PTO)]

This seems to happen with activities that provide a destination number but not a default body. We try to append the body to the newly created message and since it's null it triggers the error. The fix is a one-liner so I'm taking this right away.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH] Don't try to insert the message body if the activity didn't populate it

Simple fix with an additional unit-test included.

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 8398913 [details] [diff] [review]
[PATCH] Don't try to insert the message body if the activity didn't populate it

Review of attachment 8398913 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

thanks for this simple fix :)

::: apps/sms/js/compose.js
@@ +414,5 @@
>            this.focus();
>          }.bind(this));
>          this.ignoreEvents = true;
>        } else {
> +        this.append(message.body ? message.body : '');

maybe use the simpler construct `message.body || ''`

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #3)
> maybe use the simpler construct `message.body || ''`

I didn't use that because '' is considered false as far as logical operators go so I was unsure if it'd do what I want (i.e. replace null for '') or not. Thanks for the review!

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PULL REQUEST] Don't try to insert the message body if the activity didn't populate it

Here's the PR; waiting for Travis to turn green before merging:

https://travis-ci.org/mozilla-b2g/gaia/builds/22036849

Comment 6

[Gabriele Svelto [:gsvelto] (PTO)]

Pushed to gaia/master 20f1837dfe750a01b192f5bb9a85124e15b3809a

Comment 7

[Julien Wajsberg [:julienw]]

(In reply to Gabriele Svelto [:gsvelto] from comment #4)
> (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> #3)
> > maybe use the simpler construct `message.body || ''`
> 
> I didn't use that because '' is considered false as far as logical operators
> go so I was unsure if it'd do what I want (i.e. replace null for '') or not.
> Thanks for the review!

But your code is exactly equivalent :)

Replacing '' with '' doesn't hurt much (and your code was doing this too ;) ).