Closed
Bug 792328
Opened 12 years ago
Closed 12 years ago
B2G MMS: MMSCONF-MED-C-00{1,2,3,4,5}: Support for {us-ascii,utf-8,utf-16,iso-8859-1} as media type text
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: vicamo, Assigned: ctai)
References
Details
Attachments
(2 files, 9 obsolete files)
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
See OMA-TS-MMS-CONF-V1_3-20110913-A section 7.1.9, and Appendix C.1.12
See OMA-ETS-MMS_CON-V1_3-20101015-C section 5.1.2.1.2, 5.2.3.1.2
Reporter | ||
Comment 1•12 years ago
|
||
See also OMA-ETS-MMS_CON-V1_3-20101015-C section 5.1.2.1.1, 5.2.3.1.1, 5.2.3.1.3
Summary: B2G MMS: MMSCONF-MED-C-003: Support for utf-8 as media type text → B2G MMS: MMSCONF-MED-C-00{1,2,3,4,5}: Support for {us-ascii,utf-8,utf-16,iso-8859-1} as media type text
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → ctai
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #683833 -
Flags: feedback?(vyang)
Reporter | ||
Comment 3•12 years ago
|
||
(In reply to Chia-hung Tai [:ctai :ctai_mozilla :cht] from comment #2)
> Created attachment 683833 [details] [diff] [review]
> Add utf-16 into WellKnownCharset and unit test for this modifications
In this issue, we have to decode blob content of messages whose content-type is text/*. Depends on bug 801632 as well.
Depends on: 801632
Reporter | ||
Updated•12 years ago
|
Attachment #683833 -
Flags: feedback?(vyang)
Assignee | ||
Comment 4•12 years ago
|
||
Ths code about register converter is in intl/uconv/src/nsUConvModule.cpp.
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #683833 -
Attachment is obsolete: true
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #697810 -
Attachment is obsolete: true
Attachment #699641 -
Flags: review?(vyang)
Assignee | ||
Updated•12 years ago
|
Attachment #699641 -
Flags: review?(vyang) → review-
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #699641 -
Attachment is obsolete: true
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #700495 -
Attachment is obsolete: true
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #700504 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #700913 -
Flags: review?(vyang)
Reporter | ||
Comment 10•12 years ago
|
||
Comment on attachment 700913 [details] [diff] [review]
Support more decoder in MMS text attachment
Review of attachment 700913 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/mms/src/ril/WspPduHelper.jsm
@@ +2165,5 @@
> +
> + let str;
> + if(!entry.converter) {
> + //Todo: Check default charset.
> + entry.converter = "UTF-8";
Everytime you put a TODO in comments, it must be followed by a bug number. But, maybe we can just solve it in this bug?
@@ +2170,5 @@
> + }
> +
> + // Read a possible string quote(<Octet 127>).
> + let begin = data.offset;
> + if (Octet.decode(data) != 127) {
Everytime you put a TODO in comments, it must be followed by a bug number. But, maybe we can just solve it in this bug?
@@ +2297,3 @@
> let content = null;
> + if (headers["content-type"].media.indexOf("text/") === 0) {
> + content = StringContent.decode(data, contentEnd,
Since the StringContent.decode may throw, you'd better fallback to store it as a blob. Something like:
let octetArray = Octet.decodeMultiple(data, contentEnd);
let content = null;
if (octetArray) {
content = StringContent.decode(); // never throws
if (!content) {
content = new Blob(...);
}
}
Attachment #700913 -
Flags: review?(vyang) → review-
Reporter | ||
Comment 11•12 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #10)
> Comment on attachment 700913 [details] [diff] [review]
> Review of attachment 700913 [details] [diff] [review]:
-----------------------------------------------------------------
> ::: dom/mms/src/ril/WspPduHelper.jsm
> @@ +2170,5 @@
> > + }
> > +
> > + // Read a possible string quote(<Octet 127>).
> > + let begin = data.offset;
> > + if (Octet.decode(data) != 127) {
>
> Everytime you put a TODO in comments, it must be followed by a
> bug number. But, maybe we can just solve it in this bug?
Wrong paste. I mean, there attachment content is not a certain field in MMS/WSP binary xml. There is no string quote, actually no any format but strings encoded as utf-8/utf-16/etc.
Assignee | ||
Comment 12•12 years ago
|
||
Attachment #700913 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #701114 -
Flags: review?(vyang)
Reporter | ||
Comment 13•12 years ago
|
||
Comment on attachment 701114 [details] [diff] [review]
Support more decoder in MMS text attachment
Review of attachment 701114 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/mms/src/ril/WspPduHelper.jsm
@@ +2156,5 @@
> + let entry = WSP_WELL_KNOWN_CHARSETS[charset];
> + if (!entry) {
> + // Set converter to default one.
> + // @see OMA-TS-MMS-CONF-V1_3-20050526-D 7.1.9
> + entry.converter = "UTF-8";
entry is undefined here. Accessing 'entry.*' will trigger an exception.
@@ +2163,5 @@
> + let str;
> + if(!entry.converter) {
> + // Set converter to default one.
> + // @see OMA-TS-MMS-CONF-V1_3-20050526-D 7.1.9
> + entry.converter = "UTF-8";
You should make 'converter' an local variable here, or you'll change the values in WSP_WELL_KNOWN_CHARSETS. For example:
let entry = WSP_WELL_KNOWN_CHARSETS[charset];
let converter = (entry && entry.converter) || "UTF-8";
@@ +2166,5 @@
> + // @see OMA-TS-MMS-CONF-V1_3-20050526-D 7.1.9
> + entry.converter = "UTF-8";
> + }
> +
> + let raw = Octet.decodeMultiple(data, dataEnd);
Please move this outside the decode func as comment #10 shows.
::: dom/mms/tests/test_wsp_pdu_helper.js
@@ +1295,5 @@
> + }, raw, str);
> + }
> +
> + let (entry = WSP.WSP_WELL_KNOWN_CHARSETS["utf-16"]) {
> + // "Mozilla" in full width.
nit: intent
Attachment #701114 -
Flags: review?(vyang)
Assignee | ||
Comment 14•12 years ago
|
||
Attachment #701114 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #703201 -
Flags: review?(vyang)
Assignee | ||
Comment 15•12 years ago
|
||
Attachment #703201 -
Attachment is obsolete: true
Attachment #703201 -
Flags: review?(vyang)
Assignee | ||
Comment 16•12 years ago
|
||
Attachment #703217 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #703222 -
Flags: review?(vyang)
Reporter | ||
Comment 17•12 years ago
|
||
Comment on attachment 703222 [details] [diff] [review]
Support more decoder in MMS text attachment
Review of attachment 703222 [details] [diff] [review]:
-----------------------------------------------------------------
Simple and elegant! That's what we really want ;)
Attachment #703222 -
Flags: review?(vyang) → review+
Reporter | ||
Comment 18•12 years ago
|
||
Assignee | ||
Comment 19•12 years ago
|
||
Thanks for your review. :)
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #17)
> Comment on attachment 703222 [details] [diff] [review]
> Support more decoder in MMS text attachment
>
> Review of attachment 703222 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Simple and elegant! That's what we really want ;)
Comment 20•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•12 years ago
|
blocking-b2g: leo? → leo+
Assignee | ||
Comment 22•12 years ago
|
||
Comment 23•12 years ago
|
||
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-firefox21:
--- → fixed
Comment 24•12 years ago
|
||
The entire set of clian's pushes was backed out for multiple reasons.
https://tbpl.mozilla.org/?tree=Mozilla-B2g18&rev=a0b06192f882
1.) The tree rules are clear that you are not to land on top of bustage. At the time you pushed, both B2G Mn and B2G xpcshell had bustage from prior commits that hadn't been backed out yet.
2.) The tree rules are also clear that you are to watch your pushes for any bustage and handle them accordingly. mozilla-inbound is the ONLY tree where this rule does not apply.
3.) Even after the earlier bustage was backed out, something in one of your many pushes was causing further B2G Mn failures as shown in the log below.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20424173&tree=Mozilla-B2g18
4.) This isn't cause for backout by itself, but it is also strongly preferred to not push each commit individually as our build and testing resources are limited and doing so stretches them even thinner. Please limit your number of pushes as much as possible unless you have good reason for keeping them separate.
status-firefox20:
--- → wontfix
Reporter | ||
Comment 25•12 years ago
|
||
Reporter | ||
Updated•12 years ago
|
Blocks: mms-oma-compliance
Updated•12 years ago
|
Flags: in-moztrap-
You need to log in
before you can comment on or make changes to this bug.
Description
•