939971 - [SMS] SMS are cut if you switch App while writing your text message

Status: VERIFIED FIXED

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

Description

[Théo Chevalier]

I want to send "foo bar", but if I switch App between "Foo" and "bar", only Foo is actually sent.

STR:
1. Open SMS App
2. Type "Foo" in message field
3. Tap on Home button
4. Tap on SMS App icon
5. Add " bar" to your message
6. Tap on Send button

Actual result:
"Foo" is sent.

Expected result:

"Foo bar" is sent.


I don't think this is an edge case since it affects all the cases where you need to check another App while typing your message (i.e. checking a number, a web page), so nominating koi?.

Tested on:
ZTE Open with 1.0.1 -> unaffected
Peak with 1.1 stable from GP -> unaffected

Keon with latest 1.2 and 1.3 builds from GP -> affected
ZTE Open with up-to-date 1.2 tree -> affected

Comment 1

[Julien Wajsberg [:julienw]]

Interesting.
We got another bug filed recently, that I closed because the STR was not complete enough.

We should definitely fix this in 1.2.

Comment 2

[Sarah Parsons]

This issue started to occur on the Buri 1.2 Build ID: 20131108004004

Gaia   4cf40fb30c7b8380ea83ed0d4efe043b8b81737f
SourceStamp a886c641a306
BuildID 20131108004004
Version 26.0

Last working Buri 1.2 Build ID: 20131107004003

Gaia   590eb598aacf1e2136b2b6aca5c3124557a365ca
SourceStamp 26f1e160e696
BuildID 20131107004003
Version 26.0

Comment 3

[Julien Wajsberg [:julienw]]

The only change in SMS in this gaia hash range is 505db353e7fe24a1251de2aeb2e6c85c239243f6.

This can definitely trigger regressions but unit tests should have covered this.

Sarah or another QA person, could you please upload a logcat from when the issue is happening?

Comment 4

[Julien Wajsberg [:julienw]]

Note: I'd like a logcat covering the whole STR. Thanks!

Comment 5

[Sarah Parsons]

Attachment: SMS_Logcat

Comment 6

[Sarah Parsons]

See Logcat attached.

Comment 7

[Julien Wajsberg [:julienw]]

Thanks!

Nothing interesting though :(

Taking to investigate tomorrow.

Comment 8

[Julien Wajsberg [:julienw]]

I can't reproduce on the simulator and debugging on my phone fails now, so I'll see how to debug tomorrow.

Comment 9

[Julien Wajsberg [:julienw]]

Attachment: patch v1

* Never add an empty text in the content array
* Remove old code that is not used anymore
---
 apps/sms/js/compose.js             |   19 +++++++------------
 apps/sms/test/unit/compose_test.js |   21 ++++++++++++++-------
 2 files changed, 21 insertions(+), 19 deletions(-)

Github PR is at https://github.com/mozilla-b2g/gaia/pull/13846

Corey, I'd value your feedback too since you initially wrote this code.

I've tested various things on the device and things seems to still work fine.

Comment 10

[Corey Frang [:gnarf]]

Comment on attachment 8334586 [details] [diff] [review]
patch v1

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

LGTM

Comment 11

[Steve Chung [:steveck]]

Comment on attachment 8334586 [details] [diff] [review]
patch v1

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

The patch looks good, thanks for the fixing and removing the dead code!

Comment 12

[Julien Wajsberg [:julienw]]

master: d2b07c096385e37621700c1162cd0803ec9282f6

thanks!

Comment 13

[Julien Wajsberg [:julienw]]

The root cause is probably a small change in how contenteditable works in Gecko. And this triggered a bug in our code that I fixed here.

Triagers, I think this should go to 1.2: small patch on a well-tested component for a frustrating bug.

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 d2b07c096385e37621700c1162cd0803ec9282f6
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 15

[Julien Wajsberg [:julienw]]

If only all conflicts were so easy :)

v1.2: 0556e6b69da88df04c595fa0dbbe994dc3bbe1a0

Comment 16

[Théo Chevalier]

Verified with today's GP Keon 1.2, thanks!

Comment 17

[Lionel Mauritson]

Verified that the complete message sends when resuming a suspended message.

1.2 Environmental Variables:
Device: Buri v1.2 COM RIL
BuildID: 20131204004003
Gaia: 8d762f3376318fd6be390432db750ae4904c9ab6
Gecko: 758f3fb32dda
Version: 26.0
Firmware Version: V1.2_20131115

1.3 Master Environmental Variables
Device: Buri v1.3 COM RIL
Build ID: 20131203040236
Gecko: 8648aa476eef
Gaia: 31808a29cfcffa584b6a88b4f1e515387f485a1b
Platform Version: 28.0a1
RIL Version: 01.02.00.019.102
Firmware Version: v1.2_20131115