Closed Bug 909622 Opened 7 years ago Closed 6 years ago

[B2G][Helix][messages][zhaotao]Sending mms with attachment(chinese name mp3) fail

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect, P2, major)

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed, b2g-v1.2 fixed)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed

People

(Reporter: lecky.wanglei, Assigned: gnarf)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached image 2013-08-26-11-28-46.png
User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; aff-kingsoft-ciba; Zune 4.7)

Steps to reproduce:

1.open messages app
2.create a new message and add a mp3 file with chinese name
3.send the message


Actual results:

in step 3,sending the message failed.please check the attachment file


Expected results:

The message should be sent successfully.
Component: General → Gaia::SMS
Priority: -- → P2
blocking-b2g: --- → hd?
Severity: normal → blocker
looks like a Gecko bug
Severity: blocker → major
Component: Gaia::SMS → DOM: Device Interfaces
Flags: needinfo?(vyang)
Product: Boot2Gecko → Core
Or a carrier bug ?

Maybe we should not put non-ascii characters in the smil file ?
We have following structure for each part(attachment) in a MMS PDU:

  {
    headers: {
      content-type: {
        media: "application/foo",
        params: {
          name: <attachment.location>,
        },
      }
      content-location: <attachment.location>,
      content-id: <attachment.id>,
    },
    content: <attachment.content>,
  }

From OMA-TS-MMS_ENC-V1_3-20110913-A subclause 7.1 "Special Considerations Regarding Usage of WSP in MMS", the |name| parameter, or the <attachment.id>, MUST be encoded as "Text-value", which is basically a string consisted of characters with char code < 255.  There is a flow in WSP encoder[1] that we don't have such 255 check and may result in incompatible PDU generated.  However, we should still generate new names for attachments with non-ascii characters in their names, so that we can send such attachments out instead of format errors.

The |content-location| header field, or the <attachment.location>, has the same problem, too.

[1]: http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/WspPduHelper.jsm#478
Flags: needinfo?(vyang)
s/flow/flaw/
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #3)
> However, we should still generate new names for attachments
> with non-ascii characters in their names, so that we can send such
> attachments out instead of format errors.

BTW, Gecko can't help with such generation unless we build SMIL generator into Gecko as well.  I'll file a bug for that WSP flaw separately.
Component: DOM: Device Interfaces → Gaia::SMS
Product: Core → Boot2Gecko
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #5)
> I'll file a bug for that WSP flaw separately.

Bug 912528, if you're interested.
Thanks Vicamo.

That means we need to fix the SMIL generator that is in Gaia here, right ?
(In reply to Julien Wajsberg [:julienw] from comment #7)
> That means we need to fix the SMIL generator that is in Gaia here, right ?

Yes, thank you. :)
Steve, would you have the bandwidth to do this ?
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can take a look.
Assignee: nobody → schung
Hi Ayman,
Since it's not possible to send attachment using non-ascii character for filename, how can we solve this issue in aspect of ux? My android phone message app will replace the character with '_', some other message app might using '?' instead, or maybe we can simply replace the filename string with 'attachment'.
Flags: needinfo?(aymanmaat)
(In reply to Steve Chung [:steveck] from comment #11)
> Hi Ayman,
> Since it's not possible to send attachment using non-ascii character for
> filename, how can we solve this issue in aspect of ux? My android phone
> message app will replace the character with '_', some other message app
> might using '?' instead, or maybe we can simply replace the filename string
> with 'attachment'.

Ok a quick consultation with UX colleagues indicates that we would like to replace non-ascii characters with a '#'. Reasoning behind it is that a '_' might actually be used in a file name and a '?' is considered annoying.

ni? me if that does not work for you Steve
Flags: needinfo?(aymanmaat)
renominate this issue since hd branch is dead.
blocking-b2g: hd? → koi?
Duplicate of this bug: 919465
partners want leo for this (see bug 919465)
blocking-b2g: koi? → leo?
leo+, this impacts Greek market
blocking-b2g: leo? → leo+
Assignee: schung → gnarf37
Attached patch patch v1 (obsolete) — Splinter Review
Steve, Julien asked me to jump on this one because of it's leo+ priority.  I'm sure you would have arrived at the same solution :)
Attachment #808594 - Flags: review?(felash)
Attached patch patch v2Splinter Review
Removed the Template.escape() in the name sanitation in preference for replacing "unsafe" characters.  Also add a test to ensure that ' and " are always replaced.
Attachment #808594 - Attachment is obsolete: true
Attachment #808594 - Flags: review?(felash)
Attachment #808599 - Flags: review?(felash)
Comment on attachment 808599 [details] [diff] [review]
patch v2

r=me

thanks for the quick patch !
Attachment #808599 - Flags: review?(felash) → review+
master: https://github.com/mozilla-b2g/gaia/commit/c6625624dec07e2c875c8f3019b512f5ed2d076e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Uplifted c6625624dec07e2c875c8f3019b512f5ed2d076e to:
v1.2: 9d2150bd26ca5ff639114dfb0058d35ab871dcc0
That's a quick fix! It might be a little bit different from my original though because we have text_normalize share library with to ascii api: https://github.com/mozilla-b2g/gaia/blob/master/shared/js/text_normalizer.js#L92 . We can add replace mechanism to this api. It may help increasing the string readability for some European language.
blocking-b2g: leo+ → hd?
v1.1.0hd: 763757e133a4fa8b0cb49f35a8e6b6700c0bf345
Steve> I didn't know we had this, could be a good idea to use it in addition to the replacing (for non-latin languages).

Let's file a new bug for this for 1.3 !

(to triagers: leo+ was removed by Steve in comment 23, please add it back)
blocking-b2g: hd? → leo?
(In reply to Julien Wajsberg [:julienw] from comment #25)
> Steve> I didn't know we had this, could be a good idea to use it in addition
> to the replacing (for non-latin languages).
> 
> Let's file a new bug for this for 1.3 !
> 
> (to triagers: leo+ was removed by Steve in comment 23, please add it back)

I create a follow up (bug 919980). Sorry for the inconvenience about flag reset, but it's really annoying and you had better refresh your page before you submit changes...
blocking-b2g: leo? → leo+
re-setting leo+ per comment #25
You need to log in before you can comment on or make changes to this bug.