845173 - [MMS][Utility] Convert HTML element to mms SMIL and vise versa.

Status: RESOLVED FIXED

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

Description

[Steve Chung [:steveck]]

We need to convert the element in rich text editor into SMIL document for sending the mms and parse SMIL for necessary information to display message in the thread. We need to make sure : 
 - SMIL we create could be read on other devices and
 - SMIL sent from other devices could be displayed. (We may not need to parse every detail information for showing message, it depends on UX requirement)

Comment 1

[Joe Cheng [:jcheng] (please needinfo)]

would like to have some feedback from Bocoup on this well well

Comment 2

[Steve Chung [:steveck]]

Hi Joe, it would be great to clarify how deep Bocoup will involve in this. MMS sending  process could be divided to :
(1)HTML message content -> (2) SMIL converter -> (3) Apply send API for sending the MMS with SMIL document.
Based on the user story Bocoup will take over the rich editor part, so (1) creating a HTML message content could be ready when rich text editor landed. I will finish (3) when dom api landed. We need someone to take (2), this utility might depends on the layout of content in (1).

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

(In reply to Steve Chung from comment #2)
> Based on the user story Bocoup will take over the rich editor part, so (1)
> creating a HTML message content could be ready when rich text editor landed.
> I will finish (3) when dom api landed. We need someone to take (2), this
> utility might depends on the layout of content in (1).

Will Bocoup be able to take (2)?

Comment 4

[Joe Cheng [:jcheng] (please needinfo)]

leo+, this bug is needed to fulfill MMS user stories for v1.1

Comment 5

[Steve Chung [:steveck]]

I'll take this task first and provide utility interface for creating/parsing the SMIL few days later.

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

per discussion with Steve, by the end of this week (3/8), he will provide a more complete structure for Gaia to access MMS. Gaia need to provide the data arrays.

Structures like below will be provided first this week (3/8), the actual functions  is to be integrated with Gecko in the planned schedule of 3/11~3/15.

function smilComposor(slideArray) {
    ...
    return smil;
}

var smilDoc = smilComposor(
[{ // slide 1 : each slide page could only contain 1 media file and 1 text string
  name: file1_name,
  blob: file1_bolb,
  text: string1
},
{ // slide 2
  name: file2_name,
  blob: file2_bolb,
  text: string2    
}])

var message = new MmsMessage(number, smilDoc);
// set meesage attachement
message.attachments[name] = blob;
...

var req = navigator.mms.send(message);
...

Comment 7

[Steve Chung [:steveck]]

Attachment: mms smil document and attachment handling

Comment 8

[Steve Chung [:steveck]]

Hi all
I've updated the patch and the smil handling part is ready now(at least the createMmsMessage/send part which has been tested and sent some mms to iphone successfully). The data extraction part is also finished, but the receiving API is still reviewing and I didn't test it with real mms message yet. I'll verify the data extraction functionality when message receiving API landed. Please take a look and any feedback or comment is welcome!

Comment 9

[Steve Chung [:steveck]]

New patch added for some small fixing and message receiving ability. I've coworked with gecko devs and tested on several devices these days. This patch should work for most sending/receiving cases. Here is my testing branch: https://github.com/steveck-chung/gaia/tree/MMS-smil-testing. It add message app to oop black list and move contact workaround for MMS developing. Although the getThreadList is still not working yet, we can send a sms message to device first for entering the thread view, and send a mms message for displaying the meesage bubble. I've also add some testing code for showing the result, you can build the bubble layout based on this testing code or totally by yourself. Borja/Rick, it you are ok with this solution, we can land this patch before building up the rest part, thanks.

Comment 10

[Joe Cheng [:jcheng] (please needinfo)]

wonder what's blocking the review? thanks

Comment 11

[Joe Cheng [:jcheng] (please needinfo)]

leo+ as this is a part of MMS. No_UPLIFT for now before the whole MMS is completed

Comment 12

[Corey Frang [:gnarf]]

I have taken a look over this branch and have added some additional commits i'd like to see included, please look at https://github.com/steveck-chung/gaia/pull/3 for more details

Comment 13

[Steve Chung [:steveck]]

Thanks for the feedback. I will take a look and give a test. It looks good at the first glance. If these commits works perfectly, I will merge these changes into the testing branch for continuing your MMS developing and also current reviewing branch.

Comment 14

[Corey Frang [:gnarf]]

Thanks, if they don't work, can you send back some test cases so I can fix em?

Comment 15

[Steve Chung [:steveck]]

Hi Corey, thanks for the feedback/unit test/refactoring. I've left some comments in github but it looks great overall! I will merge it to testing branch and this reviewing branch when we finish the review with your patch.

Comment 16

[Corey Frang [:gnarf]]

No problem, thanks for getting everything started here - I think my latest commit adresses your concerns.

Comment 17

[Steve Chung [:steveck]]

Hi all,
I've update the patch with Corey's feedback. Please feel free to give any comment about the new patch here or github, thanks.

Comment 19

[Borja Salguero [:borjasalguero]]

After taking a look to this patch we have discovered a weird behaviour in Gecko which should be confirmed. The bug is here https://bugzilla.mozilla.org/show_bug.cgi?id=865157.

Comment 20

[Julien Wajsberg [:julienw]]

doing the review now, sorry for the delay,

Comment 21

[Corey Frang [:gnarf]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/9410

Comment 22

[Steve Chung [:steveck]]

Attachment: Patch for comments small fixing

Hi Corey, I add some comments and fixing in the patch, please take a look. Thanks.

Comment 23

[Julien Wajsberg [:julienw]]

Comment on attachment 742217 [details] [diff] [review]
Patch for comments small fixing

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

next time, please generate your patch with -U8

otherwise these are all good changes

::: apps/sms/js/smil.js
@@ +56,5 @@
> +  // In this case, all the media files(img1/img2/video1) and text string
> +  // (string1/string2) will be converted into 5 attachments, and SMIL should
> +  // illustrate that this mms message contains 3 slides and the attachment's
> +  // name/layout... in each slide. Here we provide parse and generate utils
> +  // for developers to focus on the necessary information in slides.

thanks for this explanation, this should definitely be in the final patch

::: apps/sms/js/thread_ui.js
@@ +501,4 @@
>            sourceTag.src = url;
>            mediaElement.appendChild(sourceTag);
>          }
> +        container.appendChild(mediaElement);

good catch

::: apps/sms/style/sms.css
@@ +394,4 @@
>    overflow: hidden;
>  }
>  
> +.mms-bubble-content > img {

Yay ;)

Comment 24

[Julien Wajsberg [:julienw]]

to drivers: we won't land today but we did another cycle of review/fixes. I think this is in very good shape to land early monday.

To whoever needs this to do some other work, you can start from Corey's branch as I don't think much will change, especially outside smil.js.

Comment 25

[Julien Wajsberg [:julienw]]

Comment on attachment 741850 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/9410

r=me

please squash, write a good commit log, and I'll merge !

thanks for this ungrateful work :)

Comment 26

[Julien Wajsberg [:julienw]]

master: ab0d1173cb6d3eb98b7a75e4d1feaa1846216612

Comment 27

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 ab0d1173cb6d3eb98b7a75e4d1feaa1846216612
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 28

[Julien Wajsberg [:julienw]]

v1-train: 6bb94463331b6a968556b4d30ea1f57b25577e0d