Closed Bug 925224 Opened 11 years ago Closed 10 years ago

[B2G][Helix][message][zhaotao]when receiving mms with mixed vcard and image content,vcard info will be abandon

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::SMS
Type:
defect

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed)

Status:
RESOLVED FIXED
Milestone:
1.3 C2/1.4 S2(17jan)
Project Flags:
blocking-b2g 1.3+
Tracking Flags:
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed

People

(Reporter: lecky.wanglei, Assigned: steveck)

References

Details

Attachments

(1 file)

Link to github
46 bytes, text/x-github-pull-request
julienw
: review+
Details | Review 
User Agent: Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; Trident/6.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:

【Detail Description*】：when receiving mms with mixed vcard and image content,vcard info will be abandon
【Repro Steps*】：
1、use another phone to send mms to ffos phone,with one jpg picture and a contact info,so the mms will be composed with image/jpeg and text/x-vCard
2、use another phone to send mms to ffos phone,with a contact info,so the mms will be composed with text/x-vCard



Actual results:

【Real Result*】：
In step1,when ffos phone receive the mms,only the picture show up,the vcard info is missing; 

In step2,when ffos phone receive the mms,the contact info will be displayed as text

【Test Count*】：5
【Found Count*】：5
【Gaia commit ID*】:8059864bd278645ab5d2598b5ed595154136c253
【Gecko commit ID*】:0265cd51b4dc64ca100353482ada2025df6f5f53
【Log*】：
【Network environment】：
【Resume operation】：
【Carrier】：


Expected results:

the vcard info should not be abandon(though vcard importing is not supported by now)
Priority: -- → P2
hi,julien

could you help check this issue?
However, it's quite strange to have 2 different behaviours whether we have 1 or 2 attachments here.

Since v-card is text/vcard or text/x-vcard, I think the code should always show the text.

bug 892358 is related, in that bug we're supposed to show a "content unsupported" image.

lecky, what is the expectd behavior for you ? Should we get the raw text, or instead should we have a "content unspported" image ?

Ayman, same question for you :) (context: we're receiving a vcard attachment, but we don't support this yet)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(aymanmaat)
hi,julien,

i think it should have same behaviour when receiving 1 vcard or mixed vcard mms.

I think raw text display is better.
Seems we can't send a vcard with Android, will try to find an iphone...

NI me to keep this bug in my radar.
Flags: needinfo?(felash)
hi,julien

vcard can be sent with android,open a contact and share it to message

then you can choose sending it by vcard.
I think it probably depends on the android flavor; on the stock android I had last week, I couldn't find it, but now I have a Samsung-branded android, so maybe I'll be able to do it. Will try today.
Flags: needinfo?(felash)
Flags: needinfo?(felash)
I could send a vcard attachment with the Samsung Contact and Message apps :)

What I found is that Samsung's Messaging app does not send similar messages whether you send only one vcard or if you send a vcard with other content.

* when sending only one vcard, Samsung's Message app is sending a smil that looks like this:

<smil>
  <head>
    <layout>
      <root-layout height="128" width="128"/>
      <region height="100%" left="0" top="0" width="100%" id="Image"/>
      <region height="100%" left="0" top="0" width="100%" id="Text"/>
    </layout>
  </head>
  <body>
    <par dur="10000ms">
      <ref src="Contact_Name.vcf" region="Image"/>
    </par>
  </body>
</smil>


* when sending a vcard + some text, Samsung's Message app is sending a smil like this:

<smil>
  <head>
    <layout>
      <root-layout height="128" width="128"/>
      <region fit="meet" height="12" left="0" top="115" width="127" id="Text"/>
    </layout>
  </head>
  <body>
    <par dur="5000ms">
      <text src="text_0.txt" region="Text"/>
    </par>
    <par dur="10000ms">
      <ref src="Contact_Name.vcf" region="Text"/>
    </par>
  </body>
</smil>

I don't exactly get why the region is different, but we ignore that information anyway.

What happens is that we normally just ignore `ref` parts, that's why we don't display the vCard in the second case.

However, we also have another mechanism: if we don't have any slide after parsing a SMIL, we fall back to another algorithm which is basically exploring all attachments. And that's why we get the vcard in the first case: since we ignore <ref> elements, we don't have any other slides, and therefore we fall back to the second algorithm.

Now, it's not quite clear to me what we're supposed to do.

Vicamo, Steve, are we supposed to support <ref> elements in SMIL for MMS v1.1 ? Should we at least get them and display the "broken" image with the file name ?
Flags: needinfo?(vyang)
Flags: needinfo?(schung)
Flags: needinfo?(felash)
hi,julien

i think <ref> elements should already be supported,because when receiving vcf only mms,

it can be handled successfully(and vcf content is shown as text).

and maybe it was caused by region preperty,in my log,in mixed mms,both in Image region.

in my logs

when receiving only vcard,the smil info:

<smil>
    <head>
        <layout>
            <region id="Image" height="100%" width="100%" left="0" top="0" fit="meet"/>
        </layout>
    </head>
    <body>
        <par dur="5000ms">
        <ref src="P_929B.VCF" region="Image"/>
        </par>
    </body>
</smil>

when receiving mixed vcard and picture,the smil info:

<smil>
    <head>
        <layout>
        <region id="Image" height="100%" width="100%" left="0" top="0" fit="meet"/>
        </layout>
    </head>
    <body>
        <par dur="5000ms">
        <img src="FOTF30A.JPG" region="Image"/>
        </par>
        <par dur="5000ms">
        <ref src="P_BA77.VCF" region="Image"/>
        </par>
    </body>
</smil>
hi,julien

i want to know if this issue be fixed on V1.1?
ref element is for the unknow type which is other than media/text. Displaying unknow icon for ref element should be the proper solution(but we don't have the ability to handle contact binary file, it still need some additional changes in message/contacts app). 

Here we have 2 different solution:
1) For the quick solution, we could apply some fixing to make the vcard display as pure text no matter the message contains other attachments or not(It seems no meaning to display a attachment but we could do nothing at all...).
2) For complete solution, we should be able to display the unknown icon and save it in sdcard at least.

Hi Wayne/Joe, if you think it's critical issue and we should fix this with first solution for quick fixing, please nominate it with blocking flag . otherwise we can take second solution in the long term, thanks.
Assignee: nobody → schung
Flags: needinfo?(wchang)
Flags: needinfo?(schung)
Flags: needinfo?(jcheng)
I'd suggest no blocking flag on this for 1.1 or 1.2, fix it correctly for the long run.
We don't claim vcard support, single or multiple. 

:wdm wdyt?
Flags: needinfo?(wchang) → needinfo?(wmathanaraj)
See Also: → 903240
hi,beatriz,

can you accept the issue?

It will not be fixed on V1.1.
Flags: needinfo?(brg)
(In reply to lecky from comment #12)
> hi,beatriz,
> 
> can you accept the issue?
> 
> It will not be fixed on V1.1.
Agree with comment 11 and nominating to have the chance to fix in master and 1.3.
blocking-b2g: --- → 1.3?
Flags: needinfo?(brg)
Just to be clear here.

This bug is _not_ about vcard support, this is another bug for this.
This bug is about what we should do when we receive unknown attachments (like vcard is right now). Our handling of unknown attachments is wrong right now.

Adding the NI flags back with this piece of information, sorry ;)
Flags: needinfo?(wchang)
Flags: needinfo?(brg)
(In reply to Julien Wajsberg [:julienw] from comment #14)
> Just to be clear here.
> 
> This bug is _not_ about vcard support, this is another bug for this.
> This bug is about what we should do when we receive unknown attachments
> (like vcard is right now). Our handling of unknown attachments is wrong
> right now.
> 
> Adding the NI flags back with this piece of information, sorry ;)

Thanks for raising the point, for the unknown type of attachment (including the vcard/vcalendar), even we claim unknown attachment download is not supported in current stage, we still need to do something to avoid this issue:

1) Hide all the unknown attachment, including vcard/vcalendar.
2) Display pure text for vcard/vcalendar within the message. But the text information will not display as same as the original encoding format(For example, sending the Chinese contact name will become hex code or other unreadable format if we treat the file as the pure text).
blocking-b2g: 1.3? → ---
blocking-b2g: --- → koi?
Flags: needinfo?(jcheng)
(In reply to Julien Wajsberg [:julienw] from comment #14)
> Just to be clear here.
> 
> This bug is _not_ about vcard support, this is another bug for this.
> This bug is about what we should do when we receive unknown attachments
> (like vcard is right now). Our handling of unknown attachments is wrong
> right now.
> 
> Adding the NI flags back with this piece of information, sorry ;)

Thanks for the additional information here Julien!
Just spoke to Joe and Steve and we'll nom this for v1.2, as we're wrapping up v1.1 now it's preferred to not add more to the scope. This here is not likely going to be of significant user impact to have to block any v1.1 release.
Flags: needinfo?(wchang)
(In reply to Wayne Chang [:wchang] from comment #16)
> (In reply to Julien Wajsberg [:julienw] from comment #14)
> > Just to be clear here.
> > 
> > This bug is _not_ about vcard support, this is another bug for this.
> > This bug is about what we should do when we receive unknown attachments
> > (like vcard is right now). Our handling of unknown attachments is wrong
> > right now.
> > 
> > Adding the NI flags back with this piece of information, sorry ;)
> 
> Thanks for the additional information here Julien!
> Just spoke to Joe and Steve and we'll nom this for v1.2, as we're wrapping
> up v1.1 now it's preferred to not add more to the scope. This here is not
> likely going to be of significant user impact to have to block any v1.1
> release.
Thanks Julien for the clarification. I agree with Wayne here. It is ok to be fixed(if possible) in 1.2.
Flags: needinfo?(brg)
 (In reply to Julien Wajsberg [:julienw] from comment #14)
> Just to be clear here.
> 
> This bug is _not_ about vcard support, this is another bug for this.
> This bug is about what we should do when we receive unknown attachments
> (like vcard is right now). Our handling of unknown attachments is wrong
> right now.
> 
> Adding the NI flags back with this piece of information, sorry ;)

from a ux pov i absolutely agree with what steve is saying in comment 10. 
> 2) For complete solution, we should be able to display the unknown icon and save it in sdcard at least.

I feel it is a fairly critical issue. The sender has paid to send the file to the recipient, and the recipient has paid to receive it …but we (if i am interpreting correctly) are failing to deliver the file or even notify of its existence. We should certainly fix asap in v1.2.
Flags: needinfo?(aymanmaat)
Helix should be nominated for 1.3. Hence moving it to 1.3?
blocking-b2g: koi? → 1.3?
Steve is the correct person for SMIL thing for we handle SMIL in Gaia completely.
Flags: needinfo?(vyang)
(In reply to ayman maat :maat from comment #18)

> I feel it is a fairly critical issue. The sender has paid to send the file
> to the recipient, and the recipient has paid to receive it …but we (if i am
> interpreting correctly) are failing to deliver the file or even notify of
> its existence. We should certainly fix asap in v1.2.

I tend to agree Ayman's comment and fix it in v1.2, but v1.2 is in string freeze stage now and it's not possible to add a new flow to show the icon and save to sdcard. Showing the pure text inside message might be solution here.

If this bug is postpone to v1.3, it still lacks of file-management-like app to handle the unknown file viewing/saving, so it's my premature proposal for this scenario: Showing the dialog with file name and size in the message content, and save/canel buttons at the bottom. Does that sound reasonable?
Steve> in 1.2, we could still at least show the file name and the "broken attachment" image, right ?
Yes we could because the "broken attachment" icon is already existed and we can use it. My concern is: if we could only shows the filename and icon in the message without saving ability, would it be better then just display the text content here?
Hi Ayman, do you have any idea if we want to fix it in 1.2? It would be great if we could show the attachment as icon and save it, but it would need your help to propose a clear flow suitable for 1.2 stage, thanks.
Flags: needinfo?(aymanmaat)
for 1.2, let's try to leave it as is
for 1.3, hope to hear UX feedback
(In reply to Steve Chung [:steveck] from comment #23)
> Yes we could because the "broken attachment" icon is already existed and we
> can use it. My concern is: if we could only shows the filename and icon in
> the message without saving ability, would it be better then just display the
> text content here?
> Hi Ayman, do you have any idea if we want to fix it in 1.2? It would be
> great if we could show the attachment as icon and save it, but it would need
> your help to propose a clear flow suitable for 1.2 stage, thanks.

I would say for V1.2, as Julien suggests, showing the file name and ‘broken attachment’ icon would be a suitable solution here as even though the file itself is not broken, the user cannot download it. So the file receipt process is ‘broken’. This would be a vast improvement in comparison to just not showing the attachment at all and form a UX perspective would be acceptable.

For V1.3 I can sketch out a proposed flow in the wireframe pack and we can move forwards with the discussion from there.

hows that sound?
Flags: needinfo?(aymanmaat)
Seems we might have more time to implement this based on comment 24. Any better idea for v1.3? Maybe we should find a way for user to save the attachment.
Now that we are quite late for 1.2 (and thus for 1.3 too), I think we won't do the attachment saving for 1.3, it's already too late, and our plate is too full. Let's do the broken image for 1.3, and have a wireframe ready early for 1.4.
(In reply to Julien Wajsberg [:julienw] from comment #27)
> Now that we are quite late for 1.2 (and thus for 1.3 too), I think we won't
> do the attachment saving for 1.3, it's already too late, and our plate is
> too full. Let's do the broken image for 1.3, and have a wireframe ready
> early for 1.4.

ok, thats good for me.
Steve, maybe we can turn this into a mentored bug, with you as mentor?
(In reply to Julien Wajsberg [:julienw] from comment #27)
> Let's do the broken image for 1.3, and have a wireframe ready early for 1.4.

triage: 1.3+ for broken image
blocking-b2g: 1.3? → 1.3+
Target Milestone: --- → 1.3 Sprint 5 - 11/22
Target Milestone: 1.3 Sprint 5 - 11/22 → ---
Flags: needinfo?(wmathanaraj)
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Hi Ayman, I got another thought about repersenting the vcard attachment. Since we also have document icon for attachment, could we use this icon for vcard(and vcalander)? Using broken image might mislead the user that attachment is broken image file, but I'm ok this that because user can do nothing with the broken files. Just let you know we have another option for this attachment(and we sould apply document icon when vcard support is ready, seems good if we can apply this icon for 1.3).
Flags: needinfo?(aymanmaat)
Target Milestone: 1.3 C2/1.4 S2(17jan) → ---
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Attached file Link to github — 
Made some fixing in smil.js for supporting vcard/vcalendar attachment. I use document icon in this patch and we might need ux feedbacck for this changes.
Attachment #8351175 - Flags: review?(felash)
Comment on attachment 8351175 [details] [review]
Link to github

works fine

r=me once you do the changes we discussed together (the unit test, and whitelisting text/plain only for text attachments)
Attachment #8351175 - Flags: review?(felash) → review+
Test cases fixed with whitelisting text/plain only in the latest patch. Thanks for the review!

Landed in master: c9f2914b584caa761f0c93d9b6b3f606fd7bafed
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: needinfo?(aymanmaat)
Resolution: --- → FIXED
status-b2g-v1.3: --- → affected
Uplifted c9f2914b584caa761f0c93d9b6b3f606fd7bafed to:
v1.3: 58ebb787bcfb2c551fd290f61dbebc47aead3a27
status-b2g-v1.3: affected → fixed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: