Closed Bug 775038 Opened 12 years ago Closed 12 years ago

B2G MMS: various small defects in WSP/MMS PDU parsers

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: vicamo, Assigned: vicamo)

References

Details

Attachments

(4 files, 3 obsolete files)

There are a few bugs/defects in WSP/MMS PDU decoder that have to be fixed/refactored before landing encoder code.
Attached patch Part 1: fix minor defects (obsolete) — Splinter Review
Attachment #643319 - Flags: review?(philipp)
Attachment #643320 - Flags: review?(philipp)
Attachment #643321 - Flags: review?(philipp)
Comment on attachment 643319 [details] [diff] [review]
Part 1: fix minor defects

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

::: dom/mms/src/ril/MmsPduHelper.jsm
@@ +1026,5 @@
>    },
>  
>    /**
>     * Check existences of all mandatory fields of a MMS message. Also sets `type`
> +   * for convient access.

"convenient"?
Attachment #643319 - Flags: review?(philipp) → review+
Comment on attachment 643320 [details] [diff] [review]
Part 2: Refactor X-Mms-Retrieve-Status decoding

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

::: dom/mms/tests/test_mms_pdu_helper.js
@@ +132,5 @@
>                    null, "NotWellKnownEncodingError");
>    // Test for normal header
>    wsp_encode_test(MMS.MmsHeader, {name: "X-Mms-Message-Type",
> +                                  value: /*MMS.MMS_PDU_TYPE_SEND_REQ*/128},
> +                  [0x80 | 0x0C, /*MMS.MMS_PDU_TYPE_SEND_REQ*/128]);

Why not use the consts here?
Attachment #643320 - Flags: review?(philipp) → review+
Attachment #643321 - Flags: review?(philipp) → review+
1. address review comment #4
2. "Date" is also a mandatory field for M-Retrieve.req PDU.
Attachment #643319 - Attachment is obsolete: true
Attachment #644264 - Flags: review?(philipp)
fix review comment #5 & clean up
Attachment #644265 - Flags: review?(philipp)
fix review comment #5
Attachment #643320 - Attachment is obsolete: true
Attachment #644266 - Flags: review?(philipp)
Attachment #644266 - Attachment description: Part 2: Refactor X-Mms-Retrieve-Status decoding : V2 → Part 3: Refactor X-Mms-Retrieve-Status decoding : V2
Update r=philikon in summary only. Thanks for your review :)
Attachment #643321 - Attachment is obsolete: true
Comment on attachment 644264 [details] [diff] [review]
Part 1: fix minor defects : V2

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

I already r+'ed, no need to ask for review again :)
Attachment #644264 - Flags: review?(philipp) → review+
Attachment #644265 - Flags: review?(philipp) → review+
Attachment #644266 - Flags: review?(philipp) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: