877141 - [SMS]If you send several spaces together in a sms, the other dipositive receives a "*" for each space

Status: VERIFIED FIXED

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

Description

[rafael.marquez]

Branch:V1-train
Gecko:489c6ee
Gaia:53055a5

Procedure:
Opem SMS app
Send a sms with the text "1 2     3" to a other device

Expected Result: The other device receive a sms with te text "1 2     3"

Actual Result: The other devices receive the sms with the text "1 2*****3"

Comment 1

[Corey Frang [:gnarf]]

I can't reproduce this, but the keyboard while I was testing was replacing all my spaces with periods? *shrug*

Comment 2

[Dietrich Ayala (:dietrich)]

QA, can you reproduce on v1-train?

Comment 3

[Corey Frang [:gnarf]]

works for me, tested sending "test    this    thing" to me and dietrich, we all got the correct message - I did however figure out that double tapping space replaces with ". " -- maybe this was being broken/fixed in the keyboard app while you tested?

Comment 4

[Maria Angeles Oteo (:oteo)]

Corey, Have you tried to reproduce it with the same steps that Rafael has provided in the STR?
Using numbers? I am reproducing it in v1-train.

One suggestion: do not press the spaces among the numbers quickly as the predictive text can include other characters.

Rafa, confirm you have reproduced it in V1-train and reopen it in that case

Comment 5

[Corey Frang [:gnarf]]

I can confirm worksforme on v1-train with "1   2   3   4" as a message - sent to myself and dietrich was received properly- even "1 2     3" worked

Comment 6

[Angela Hubenya]

I was not able to reproduce this issue on the latest v1.1 build using the STR in comment 0. Numbers and spacing came through as typed. 
Tested on the Unagi and Leo device 

Build ID: 20130529070208
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/6ca32ed2bbc6
Gaia: 8f5ab7bfd4a2921aab4e2de11e0d79a29c1bb062

Comment 7

[rafael.marquez]

The bug still playing today in V1-train:
Gecko: 1287520
Gaia: 0bfcaf8
RIL: Moz Ril

Comment 8

[rafael.marquez]

Attachment: sms with several spaces

Comment 9

[Maria Angeles Oteo (:oteo)]

It's really strange... 
Corey, Angela, which RIL are you using? Mozilla or Qualcom?
We are using the Mozilla one as we are having crashes when seinding SMSs with the Qualcom one... 

In case you are using Qualcom RIL, which version? just to confirm we are not building with the correct one.

Comment 10

[rafael.marquez]

The problem is as follows:
You you can not reproduce the bug because your SIM is not activated 7bit encoding.

For example we tested with the same device, same build, same RIL with a sim O2 (UK) and a Movistar sim (Spain).
With O2 sim worked correctly, but with the Movistar SIM have detected the bug.

Comment 11

[Corey Frang [:gnarf]]

I'm unsure which RIL I use (i'm assuming mozilla? on the unagi), but this is starting to sound like an interface layer problem.  ctai? any thoughts?

Comment 12

[Maria Angeles Oteo (:oteo)]

or perhaps an encoding issue, as Rafa has pointed out...
Requesting needinfo to Vicamo as he's an expert in this area.

Comment 13

[Angela Hubenya]

I am using the Mozilla RIL

Comment 14

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

I think vyang is the right person.

Comment 15

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

Attachment: test case

This patch is to prove Mozilla RIL works correctly with reported cases.  Try: https://tbpl.mozilla.org/?tree=Try&rev=f58f0ea90129

Comment 16

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

(In reply to rafael.marquez from comment #7)
> The bug still playing today in V1-train:
> Gecko: 1287520
> Gaia: 0bfcaf8
> RIL: Moz Ril

Could you help capturing some logs? Maybe you're sending full-width white space characters or something similar.

  adb remount 
  adb pull /system/b2g/defaults/pref/user.js user.js 
  [update user.js with adding 'pref("ril.debugging.enabled", true);'] 
  adb push user.js /system/b2g/defaults/pref/user.js 
  adb shell reboot

Comment 17

[rafael.marquez]

Attachment: Log

Comment 18

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

Comment on attachment 756439 [details]
Log

This log file doesn't contain debug information for RIL.  Please try again and have full log (begins at boot) captured.

Comment 19

[rafael.marquez]

Attachment: Log complete

Comment 20

[Dietrich Ayala (:dietrich)]

To clarify for blocking - is this occurring with sim card and encoding of any of our target markets?

Comment 21

[Maria Angeles Oteo (:oteo)]

Spain is target market for v1.1 and it uses 7-bits encoding, in fact we have reported the bug using spanish sim cards so we need to set the bug leo+. Besides this a regression after MMS landing because it works for v1.0.1

I am nearly 100% sure that Brazil uses also 7-bits encoding. Requesting needinfo to Beatriz to confirm it.

Comment 22

[Beatriz Rodríguez [:brg]]

(In reply to Maria Angeles Oteo (:oteo) from comment #21)
> Spain is target market for v1.1 and it uses 7-bits encoding, in fact we have
> reported the bug using spanish sim cards so we need to set the bug leo+.
> Besides this a regression after MMS landing because it works for v1.0.1
> 
> I am nearly 100% sure that Brazil uses also 7-bits encoding. Requesting
> needinfo to Beatriz to confirm it.
All the target markets for 1.1 are using 7-bits encoding

Comment 23

[ayman maat :maat]

hi guys

I have three SIM cards at my disposal here in Germany all tied to the German network: PAYG O2, PAYG T-Mobile and a Contract T-mobile. I cannot reproduce the receiving of a text where multiple spaces next to each other are replaced by asterisks, but what i am finding is that if i have my PAYG O2 card in my Unagi device and type "a b     c"  what is output in the message field as i compose the message is "a b….. c". 

This is only happening with the PAYG O2 SIM in the phone, not with any of the others.

Build: 20130603100120

Comment 24

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

(In reply to ayman maat :maat from comment #23)
> This is only happening with the PAYG O2 SIM in the phone, not with any of
> the others.

Could you help capturing some logs as well?  Somehow directly edit user.js doesn't always work so Rafael's second capture still contains no RIL debug messages.

Comment 25

[rafael.marquez]

This week I'm executing "notification server" tests and I have no free time. For the last logs sent, I followed your steps from comment 16. Can you progress the bug while I get free time?

Comment 26

[Gene Lian (I already quit Mozilla)]

Comment on attachment 756428 [details] [diff] [review]
test case

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

::: dom/mobilemessage/tests/marionette/test_strict_7bit_encoding.js
@@ +187,5 @@
>  }
>  
> +// Bug 877141 - If you send several spaces together in a sms, the other
> +//              dipositive receives a "*" for each space.
> +function testBug877141() {

Could you please move this function to:

tasks.push(function testBug877141(){
  ...
});

which makes more clear to me since it's nothing but a test case instead of a utility function being used everywhere.

r=me with this fix. Nice patch!

@@ +214,5 @@
> +// When strict7BitEncoding is enabled, we should replace characters that
> +// can't be encoded with GSM 7-Bit alphabets with '*'.
> +tasks.push(function () {
> +  // "Happy New Year" in Chinese.
> +  let sent = "\u65b0\u5e74\u5feb\u6a02", received = "****";

新年快樂

Comment 27

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

(In reply to Gene Lian [:gene] from comment #26)
> Review of attachment 756428 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/mobilemessage/tests/marionette/test_strict_7bit_encoding.js
> @@ +187,5 @@
> >  }
> >  
> > +// Bug 877141 - If you send several spaces together in a sms, the other
> > +//              dipositive receives a "*" for each space.
> > +function testBug877141() {
> 
> Could you please move this function to:

This function is called twice to ensure no matter strict7BitEncoding is enabled or not, the continuous SP characters are not transformed to multiple '*' chars.  Per IRC discuss, will address this in comments instead.

Comment 28

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

Attachment: test case : v2

Add comments only.

Comment 29

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

https://hg.mozilla.org/projects/birch/rev/307d7abbf1c0

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/307d7abbf1c0

Comment 31

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

The same test_strict_7bit_encoding.js script runs and returns all success in:

v1.0.1:
Gecko: 0f2ddf1cf74314ff024c78467dcb8a48e323b36f

v1-train:
Gecko: 7dfb69fde6292ae53b9558f26f8f117cf84e8891

m-c:
Gecko: since landed in c9d308fc3fed43e72efad33eb17cb5438dcc2f1a

Comment 32

[Corey Frang [:gnarf]]

Is this being left open for confirmation?

Comment 33

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

(In reply to Corey Frang [:gnarf] [:gnarf37] from comment #32)
> Is this being left open for confirmation?

Rafael & Ayman reported the issue does exist, but just need some more input/logs so that we can identify the root problem.

Comment 34

[:gerard-majax]

I've just tried to reproduce unsuccessfully on my Nexus S running gecko b2g18 at b518de0 and gaia master at 5ffca88

Comment 35

[rafael.marquez]

We reproduce the bug with SIM from Movistar Spain.
Alexandre LISSY, what SIM you're using?

Comment 36

[:gerard-majax]

French Free Mobile.

Comment 37

[Beatriz Rodríguez [:brg]]

(In reply to Alexandre LISSY :gerard-majax from comment #36)
> French Free Mobile.
The "enableStrict7BitEncodingForSms" parameter is not activated for ths SIM card, this is the reason why you cannot reproduce it. Not all the carriers had this parameter activated.

Rafael Marquez will try to provide logs within today activating the suitable flags.

Comment 38

[ayman maat :maat]

Attachment: ayman_log_cat_traces_2013_06_11_v1.0

I can still reproduce this bug as detailed in Comment 23
Log attached

build: 20130611093748

NeedInfo me if you require further information from me

Comment 39

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

(In reply to ayman maat :maat from comment #38)
> Created attachment 760798 [details]
> ayman_log_cat_traces_2013_06_11_v1.0

Still have no RIL debug lines.  But thanks Edgar Chen, in bug 820699 comment 5, Shian-Yow found there are still other preference settings file that may override values in user.js.  Could you help capturing some log for this again?  Thank you :)

Comment 40

[:gerard-majax]

(In reply to Beatriz Rodríguez [:brg] from comment #37)
> (In reply to Alexandre LISSY :gerard-majax from comment #36)
> > French Free Mobile.
> The "enableStrict7BitEncodingForSms" parameter is not activated for ths SIM
> card, this is the reason why you cannot reproduce it. Not all the carriers
> had this parameter activated.
> 
> Rafael Marquez will try to provide logs within today activating the suitable
> flags.

I'm enabling RIL debug and forcing this parameter as true for my provider, we'll see :)

Comment 41

[:gerard-majax]

After forcing the parameter, I'm reproducing the issue on my Inari.

Comment 42

[:gerard-majax]

According to the logs I see, it might be gaia:

1465 I/Gecko   (  542): RIL Worker: Received DOM message {"dcs":0,"encodedFullBodyLength":13,"userDataHeaderLength":0,"langIndex":0,"langShiftIndex":0,"segmentMaxSeq":1,"segmentChars":160,"segments"       :[{"body":"1 2******** 3","encodedBodyLength":13}],"rilMessageType":"sendSMS","number":"xxxxx","requestStatusReport":true,"envelopeId":1}

Comment 43

[:gerard-majax]

I've also noticed some strange behavior: typing "1 2  3    4", the SMS is sent but only "1 2 " appears in the message list, and on the recipient side, I get "1 2*".

Comment 44

[:gerard-majax]

Just after tapping "Send":
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with (1)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now (1)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=49
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=32
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with (2)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now (2)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=50
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=-1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet(langShift)=-1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet < 0
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: reset inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=32
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets


When message get sent:
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with (1)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now (1)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=49
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=32
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with (2)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now (2)
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=50
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=-1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet(langShift)=-1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet < 0
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: reset inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: dealing with ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: now ( )
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: septet=32
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: inc=1
I/Gecko   (  790): -*- RadioInterfaceLayer: _fragmentText7Bit: <= segmentSeptets


Corresponding changes:
diff --git a/dom/system/gonk/RadioInterfaceLayer.js b/dom/system/gonk/RadioInterfaceLayer.js
index 0404bd8..be6ced2 100644
--- a/dom/system/gonk/RadioInterfaceLayer.js
+++ b/dom/system/gonk/RadioInterfaceLayer.js
@@ -2445,25 +2445,33 @@ RadioInterfaceLayer.prototype = {
     let body = "", len = 0;
     for (let i = 0, inc = 0; i < text.length; i++) {
       let c = text.charAt(i);
+      debug("_fragmentText7Bit: dealing with (" + c +")");
       if (strict7BitEncoding) {
         c = RIL.GSM_SMS_STRICT_7BIT_CHARMAP[c] || c;
+        debug("_fragmentText7Bit: now (" + c +")");
       }
 
       let septet = langTable.indexOf(c);
+      debug("_fragmentText7Bit: septet=" + septet);
       if (septet == RIL.PDU_NL_EXTENDED_ESCAPE) {
+        debug("_fragmentText7Bit: continue");
         continue;
       }
 
       if (septet >= 0) {
+        debug("_fragmentText7Bit: inc=1");
         inc = 1;
       } else {
         septet = langShiftTable.indexOf(c);
+        debug("_fragmentText7Bit: septet(langShift)=" + septet);
         if (septet == RIL.PDU_NL_RESERVED_CONTROL) {
+          debug("_fragmentText7Bit: septet(langShift) continue");
           continue;
         }
 
         inc = 2;
         if (septet < 0) {
+          debug("_fragmentText7Bit: septet < 0");
           if (!strict7BitEncoding) {
             throw new Error("Given text cannot be encoded with GSM 7-bit Alphabet!");
           }
@@ -2472,12 +2480,14 @@ RadioInterfaceLayer.prototype = {
           // characters that can't be encoded with GSM 7-Bit alphabets with '*'.
           c = '*';
           if (langTable.indexOf(c) >= 0) {
+           debug("_fragmentText7Bit: reset inc=1");
             inc = 1;
           }
         }
       }
 
       if ((len + inc) > segmentSeptets) {
+        debug("_fragmentText7Bit: > segmentSeptets");
         ret.push({
           body: body,
           encodedBodyLength: len,
@@ -2485,6 +2495,7 @@ RadioInterfaceLayer.prototype = {
         body = c;
         len = inc;
       } else {
+        debug("_fragmentText7Bit: <= segmentSeptets");
         body += c;
         len += inc;
       }

Comment 45

[:gerard-majax]

The previous comment was for sending "1 2  3"

Comment 46

[:gerard-majax]

In Gaia's SMS app,

E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:401 in mm_send: sendSMS::"1 2 � 3"
E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:403 in mm_send: sendSMS:0::1 -> 49
E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:403 in mm_send: sendSMS:1::  -> 32
E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:403 in mm_send: sendSMS:2::2 -> 50
E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:403 in mm_send: sendSMS:3::  -> 32
E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:403 in mm_send: sendSMS:4::� -> 160
E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:403 in mm_send: sendSMS:5::  -> 32
E/GeckoConsole( 1966): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:403 in mm_send: sendSMS:6::3 -> 51

Comment 47

[rafael.marquez]

Thanks for helping with the logs

Comment 48

[:gerard-majax]

I have a FIX!

Comment 49

[:gerard-majax]

Attachment: Removing non-break spaces from contenteditable

Please find attached a link to the github pull request https://github.com/mozilla-b2g/gaia/pull/10333 which addresses the issue

Comment 50

[Julien Wajsberg [:julienw]]

Comment on attachment 761303 [details]
Removing non-break spaces from contenteditable

r=me

thanks

Comment 51

[Julien Wajsberg [:julienw]]

master: 5db84b067d58b4825d4900cb66a0d10e0522a831

Comment 52

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

Uplifted 5db84b067d58b4825d4900cb66a0d10e0522a831 to:
v1-train: 19ed167c46f6633395a9726e358bcc8fe0f4233f

Comment 53

[isabelrios]

This has been verified for SMS, it is working fine now with today's build (06/13):
Gecko-fb33bb3
Gaia-6627a1e

But for MMS there are still problems when typing more than one space, so a new bug has been open to follow that up: bug 882607

Comment 54

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

1.1hd: 19ed167c46f6633395a9726e358bcc8fe0f4233f