Bug 826614 - [SMS] Refine characters counter and indicator in SMS application #7316

Merged
merged 1 commit into from Jan 8, 2013

Projects

None yet

2 participants

@borjasalguero
Mozilla-B2G member

Testing! I will come back with the review asap!

@borjasalguero borjasalguero commented on the diff Jan 7, 2013
apps/sms/js/sms.js
@@ -775,12 +772,6 @@ var ThreadUI = {
this.editForm.addEventListener('submit', this);
this.telForm.addEventListener('submit', this);
this.sendForm.addEventListener('submit', this);
-
- var self = this;
- SettingsListener.observe('ril.sms.strict7BitEncoding.enabled',
@borjasalguero
borjasalguero Jan 7, 2013

I've talked with @vingtetun and we should keep it. So we keep it!

@borjasalguero borjasalguero commented on the diff Jan 7, 2013
apps/sms/js/sms.js
- // A sms can hold 140 bytes of data or 134 bytes of data depending
- // if it is a single or concatenated sms. To be fun the numbers of
- // sms also depends on the character encoding of the message.
- if (this.is7BitEncoding || this.has7BitOnlyCharacters(value)) {
- var kMaxCharsIfSingle = 160; // (140 * 8) / 7 = 160.
- var kMaxCharsIfMultiple = 153; // ((140 - 6) / 7 ~= 153.
- } else {
- var kMaxCharsIfSingle = 70; // (140 * 8) / 16 = 70.
- var kMaxCharsIfMultiple = 67; // ((140 - 6) / 16 = 67.
- }
-
+ // We set maximum concatenated number of our SMS app to 10 based on:
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=813686#c0
+ var kMaxConcatenatedMessages = 10;
+
+ // Use backend api for precise sms segmetation information.
@borjasalguero borjasalguero commented on the diff Jan 7, 2013
apps/sms/js/sms.js
- // if it is a single or concatenated sms. To be fun the numbers of
- // sms also depends on the character encoding of the message.
- if (this.is7BitEncoding || this.has7BitOnlyCharacters(value)) {
- var kMaxCharsIfSingle = 160; // (140 * 8) / 7 = 160.
- var kMaxCharsIfMultiple = 153; // ((140 - 6) / 7 ~= 153.
- } else {
- var kMaxCharsIfSingle = 70; // (140 * 8) / 16 = 70.
- var kMaxCharsIfMultiple = 67; // ((140 - 6) / 16 = 67.
- }
-
+ // We set maximum concatenated number of our SMS app to 10 based on:
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=813686#c0
+ var kMaxConcatenatedMessages = 10;
+
+ // Use backend api for precise sms segmetation information.
+ var smsInfo = navigator.mozSms.getSegmentInfoForText(value);
@borjasalguero
borjasalguero Jan 7, 2013

Here we should check at the beginning it there is SIM Card, because without SIM Card we get the following error:

E/GeckoConsole( 399): [JavaScript Error: "TypeError: navigator.mozSms.getSegmentInfoForText is not a function" {file: "app://sms.gaiamobile.org/js/sms.js" line: 814}]

Probably first line in updateCounter should be something like
if(!simAvailable) return;

For checking simAvailable I dont know if there is a param to be requested directly of mozMobileConnection, but asking for 'pin' results in an error if there is no SIM Card.

@borjasalguero
borjasalguero Jan 7, 2013

In fact, with my Spanish SIM Card and Gecko-2825edd I have the same....

E/GeckoConsole( 397): [JavaScript Error: "TypeError: navigator.mozSms.getSegmentInfoForText is not a function" {file: "app://sms.gaiamobile.org/js/sms.js" line: 814}]

Should I need to update Gecko or something?

@borjasalguero
borjasalguero Jan 7, 2013

Ok, the Gecko part is not in my Gecko yet! Im compiling it again and I will come back with feedback ;)

@borjasalguero
borjasalguero Jan 7, 2013

Cool! After testing it with latest Gecko this part is working properly! ;)

@borjasalguero
Mozilla-B2G member

Well, brief summary. The code looks nice, and it's working properly but one issue.

Here:
https://github.com/steveck-chung/gaia/blob/5a34df7190e36bd4ef97310fc68b80a9f19c13f5/apps/sms/js/sms.js#L777

There is a bug that it's easily reproducible:

  • Type directly to 'contactInput' field, and we are testing 'updateCounter' despite of it's empty!

I've modified the code a little bit, here you have the PR to your branch! steveck-chung#2

Thanks a lot for your effort! Tomorrow morning I will try the code again with your latest changes.

@steveck-chung

@borjasalguero : Thanks for you feedback! I also add some modification in the new PR:

1) I added counter clear in cleanFields (https://github.com/steveck-chung/gaia/blob/sms-CharCounter/apps/sms/js/sms.js#L1175 ), or you will see the counter still exist after message sent.
2) Use cleanFields (https://github.com/steveck-chung/gaia/blob/sms-CharCounter/apps/sms/js/sms.js#L88). You've done something to reset for new thread page, but it still not the correct result here (you can type a lot of words in the message input, and back - > enter the new thread again to see the result in the previous build) . I reuse the cleanFields here.

If you think the fixing is inappropriate, please feel free to leave any comment. Thanks.

@borjasalguero
Mozilla-B2G member

R+! ;)

@borjasalguero
Mozilla-B2G member

Ah! Please update the commit with bug number and reviewer! Thanks! Gracias ;)

@borjasalguero borjasalguero merged commit 0a31d94 into mozilla-b2g:master Jan 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment