Closed Bug 958445 Opened 6 years ago Closed 6 years ago

[CDMA] support for Shift-JIS decoding of SMS in CDMA.

Categories

(Firefox OS Graveyard :: RIL, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
1.3 C3/1.4 S3(31jan)

People

(Reporter: na-kobayashi, Assigned: na-kobayashi)

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131205075310

Steps to reproduce:

support for Shift-JIS decoding of SMS msssage body in CDMA.
Attached patch shift-jis.patch (obsolete) — Splinter Review
I have created patch and uploaded.
Please check it.
Attachment #8358316 - Flags: review?(chulee)
Component: General → RIL
Comment on attachment 8358316 [details] [diff] [review]
shift-jis.patch

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

Looks good to me, ask review from peer.
Attachment #8358316 - Flags: review?(vyang)
Attachment #8358316 - Flags: review?(chulee)
Attachment #8358316 - Flags: review+
Comment on attachment 8358316 [details] [diff] [review]
shift-jis.patch

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

Thank you. Please have an revise for correct HG patch commit summary format "Bug NNN: <summary>. r=chulee,vicamo".
Attachment #8358316 - Flags: review?(vyang) → review+
Comment on attachment 8358316 [details] [diff] [review]
shift-jis.patch

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

::: dom/system/gonk/tests/test_ril_worker_sms_cdmapduhelper.js
@@ +77,5 @@
> + *  - encoding by shift-jis
> + */
> +add_test(function test_CdmaPDUHelper_decodeCdmaPDUMsg_Shift_jis() {
> +  let worker = newWorker({
> +    postRILMessage: function fakePostRILMessage(data) {

nit: from bug 934125 on, just have "postRILMessage: function(data) {" instead.

@@ +80,5 @@
> +  let worker = newWorker({
> +    postRILMessage: function fakePostRILMessage(data) {
> +      // Do nothing
> +    },
> +    postMessage: function fakePostMessage(message) {

ditto.
Hi Vicamo,

Thank you for review.

I have created patch and uploaded.
 - remove redundant js function name.

Please check it.
Attachment #8359652 - Flags: review?(vyang)
Comment on attachment 8359652 [details] [diff] [review]
Shift-JIS_r2.patch

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

Thank you :)
Attachment #8359652 - Flags: review?(vyang) → review+
I don't have permission to commit.
Please commit it.
Keywords: checkin-needed
Attachment #8358316 - Attachment is obsolete: true
https://hg.mozilla.org/integration/b2g-inbound/rev/f3dd3cbe2d2e
Assignee: nobody → na-kobayashi
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f3dd3cbe2d2e
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
You need to log in before you can comment on or make changes to this bug.