Created attachment 678610[details][diff][review]
Patch for SMS OA
Thanks for the review, I modify the code for your suggestion.
Except the question "Do we really need an address that contains MMI? If we just discard it here, will this cause any nonconformance?"
I think we need to keep MMI related information in this stage. Because MMI is for control the service, discard it might mislead the behaviour.

Created attachment 679034[details][diff][review]
FIx indentations
Thanks for your feedback.
I already fix the wrong indentations.
For regular expression part, I think it is right.
Let me explain it by example.
I use /(^[#*])(.*)([#*])(.*)$/ to check these cases.
1. *21#
The result[1] = *, result[2] = 21, result[3] = #, result[4] = ""
2. #21#
The result[1] = #, result[2] = 21, result[3] = #, result[4] = ""
Case 1 and 2 don't match result[2].match(/(.+)([#*])(.+)$/). So just append the + after result[3].
3. *#21#
The result[1] = *, result[2] = #21, result[3] = #, result[4] = ""
Case 3 don't match result[2].match(/(.+)([#*])(.+)$/). So just append the + after result[3].
4. *31#10123456789
The result[1] = *, result[2] = 31, result[3] = #, result[4] = "10123456789"
5. #31#+18901234567
The result[1] = #, result[2] = 31, result[3] = #, result[4] = "18901234567"
Case 4 and 5 don't match result[2].match(/(.+)([#*])(.+)$/). So just append the + after result[3].
6. 18881234567#
The result is null. Insert '+' before the address.
7. **21*886288888888#
The result[1] = *, result[2] = *21*886288888888, result[3] = #, result[4] = ""
Case 7 match result[2].match(/(.+)([#*])(.+)$/). So append '+' before result2[3].
I guess that is the tricky part.
Please let me know if you have any question about the regular expression.
For MMI code removal part, I use an android phone to set call forward by MMI code. After that step, no phone number shows in call log.
I don't think end user can send a message which OA with MMI code.
So the use case should an operator or some service provider insert the MMI code to OA for special reason. For example, maybe the operator can send a message which OA is ##02# to cancel all call forwarding. If we press the OA button in that message, it should dial the number directly to cancel all call forwarding.
If we remove the MMI code, that sounds weird for me.
And the OA from operator or some service should not match in your contacts. So I think in match contacts, we can skip all MMI code starting number. Not just discard MMI part.
Thanks

Created attachment 679531[details][diff][review]
OA without mmi code
Please review this patch.
MMI code will show not only TOA = international but also other case.
Please check the ril_worker and test case.
Thanks

Comment on attachment 679531[details][diff][review]
OA without mmi code
Review of attachment 679531[details][diff][review]:
-----------------------------------------------------------------
I've talked to Fernando, author of MMI works. Actually, both of us don't really think MMI code in DA is ever possible. These days I've been trying to find some use cases that utilize MMI codes in SMS DA but in vain (of course we're all new to this field and our reach could be pretty limited). Besides, I think these MMI parse code is mainly come from Android's shared address parsing implementation. MMI codes exist in call addresses, but it doesn't follow that SMS DA has the same thing. 3GPP TS 23.040 doesn't mention it, neither do 27.004 and 27.005. Third, there is already a more accurate/complete _parseMMI() helper function written by Fernando right in ril_worker. Should we find any use case that depends on MMI in SMS DA, we can still easily support it with that helper function. So, I'm sorry this review process takes so much time, but I'd like you to remove MMI parsing parts and simply read the normal alphanumeric address.

Created attachment 680499[details][diff][review]
No MMI parsing patch
Thanks for reviewing. I upload the patch which only dealing with alpha-numeric and append the '+' when TOA is international. You are right. Maybe there is no use case for a OA with MMI code.
By the way, for bug 758658 - B2G SMS: test scripts for GSM address parsing, I think I also solved this bug, and I right?
If yes, maybe I can take this bug. And mark as duplicated to 788441.
Thanks

Created attachment 730573[details][diff][review]
[b2g18_v1_0_1 and b2g18] patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 836383
User impact if declined: SMS with alphanumeric sender address is not received.
Testing completed: b2g18, b2g18_v1_0_1
Risk to taking this patch (and alternatives if risky): already in m-c since mozilla-19 and no any known follow-ups
String or UUID changes made by this patch: (none)