872219 - B2G MMS: "lastSegment is undefined" in RadioInterfaceLayer.js" L2544

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Rick Waldron [:rwaldron]]

adb logcat reports this several times while running the SMS app:

E/GeckoConsole(  578): [JavaScript Error: "lastSegment is undefined" {file: "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line: 2544}]



https://hg.mozilla.org/releases/mozilla-b2g18/file/0e391e36361a/dom/system/gonk/RadioInterfaceLayer.js#l2544

Comment 1

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

This API is supported to be called multiple times, and it seems we missed something when the input sting is empty.  This shouldn't bring visible errors or anything that might have bad effects to user experience. Not nominating for leo+/tef+.

Comment 2

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

Attachment: Part 1/2: fix

Comment 3

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

Attachment: Part 2/2: marionette test cases

Try: https://tbpl.mozilla.org/?tree=Try&rev=8cc5cd773ee4

Comment 4

[Gene Lian (I already quit Mozilla)]

Comment on attachment 752681 [details] [diff] [review]
Part 1/2: fix

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

Good!

Comment 5

[Gene Lian (I already quit Mozilla)]

Comment on attachment 752683 [details] [diff] [review]
Part 2/2: marionette test cases

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

::: dom/mobilemessage/tests/marionette/test_getsegmentinfofortext.js
@@ +87,5 @@
> +// Testing empty object.  The empty object extends to "[object Object]" and both
> +// '[' and ']' are in default single shift table, so each of them takes two
> +// septets.
> +addTest({}, 1, PDU_MAX_USER_DATA_7BIT,
> +        PDU_MAX_USER_DATA_7BIT - (("" + {}).length + 2));

I tried to input

"" + {}

in my JS console. It shows:

"[object Object]"

why do you want to add 2?

@@ +94,5 @@
> +let date = new Date();
> +addTest(date, 1, PDU_MAX_USER_DATA_7BIT,
> +        PDU_MAX_USER_DATA_7BIT - ("" + date).length);
> +
> +addTest("", 0, PDU_MAX_USER_DATA_7BIT, PDU_MAX_USER_DATA_7BIT);

PDU_MAX_USER_DATA_7BIT - "".length

looks more clear to me.

Comment 6

[Gene Lian (I already quit Mozilla)]

Comment on attachment 752683 [details] [diff] [review]
Part 2/2: marionette test cases

Sorry, I misunderstood. :)

Comment 7

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

Attachment: Part 2/2: marionette test cases v2

Address review comment.  Also removes unused constants PDU_MAX_USER_DATA_8BIT and PDU_MAX_USER_DATA_UCS2.

Comment 8

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

https://hg.mozilla.org/projects/birch/rev/61199c344545
https://hg.mozilla.org/projects/birch/rev/93d20e12ec97

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/61199c344545
https://hg.mozilla.org/mozilla-central/rev/93d20e12ec97

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 752681 [details] [diff] [review]
Part 1/2: fix

requesting approval because this clobbers our logs and make debugging more difficult.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: no real user impact afaik but this is called each time the user types a letter in a sms. AFAIK the error comes when we display an empty SMS.
Testing completed: yes + marionette tests
Risk to taking this patch (and alternatives if risky): low 
String or UUID changes made by this patch: none

Comment 11

[Gene Lian (I already quit Mozilla)]

+1. That error message is really annoying. :(

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/3f4dbac52492
https://hg.mozilla.org/releases/mozilla-b2g18/rev/f4292b41ff61