Closed Bug 845173 Opened 7 years ago Closed 7 years ago

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

Categories

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

x86_64
Linux
defect

Tracking

(blocking-b2g:leo+, b2g18 fixed)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
b2g18 --- fixed

People

(Reporter: steveck, Assigned: steveck)

References

Details

Attachments

(2 files, 1 obsolete file)

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)
would like to have some feedback from Bocoup on this well well
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).
(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)?
Flags: needinfo?(boaz)
leo+, this bug is needed to fulfill MMS user stories for v1.1
blocking-b2g: --- → leo+
I'll take this task first and provide utility interface for creating/parsing the SMIL few days later.
Assignee: nobody → schung
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);
...
Whiteboard: [3/11~3/15]
Attachment #722108 - Flags: review?(waldron.rick)
Attachment #722108 - Flags: review?(fbsc)
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!
Depends on: 849741
Attachment #722108 - Attachment description: Patch 1: Structure for mms smil document and attachment handling → mms smil document and attachment handling
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.
wonder what's blocking the review? thanks
Flags: needinfo?(boaz) → needinfo?(fbsc)
Flags: needinfo?(waldron.rick)
Whiteboard: [3/11~3/15]
leo+ as this is a part of MMS. No_UPLIFT for now before the whole MMS is completed
Whiteboard: NO_UPLIFT
Whiteboard: NO_UPLIFT → [NO_UPLIFT]
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
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.
Thanks, if they don't work, can you send back some test cases so I can fix em?
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.
No problem, thanks for getting everything started here - I think my latest commit adresses your concerns.
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.
Depends on: 861095
Duplicate of this bug: 859218
Attachment #722108 - Flags: review?(felash)
Priority: -- → P2
Depends on: 865157
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.
Flags: needinfo?(fbsc)
doing the review now, sorry for the delay,
Depends on: 865355
Attachment #741850 - Flags: review?(schung)
Attachment #741850 - Flags: review?(felash)
Flags: needinfo?(waldron.rick)
Attachment #722108 - Flags: review?(waldron.rick)
Attachment #722108 - Flags: review?(felash)
Attachment #722108 - Flags: review?(fbsc)
Attachment #722108 - Attachment is obsolete: true
Hi Corey, I add some comments and fixing in the patch, please take a look. Thanks.
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 ;)
Attachment #742217 - Flags: feedback+
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.
Depends on: 865273
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 :)
Attachment #741850 - Flags: review?(schung)
Attachment #741850 - Flags: review?(felash)
Attachment #741850 - Flags: review+
master: ab0d1173cb6d3eb98b7a75e4d1feaa1846216612
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [NO_UPLIFT]
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
Flags: needinfo?(felash)
v1-train: 6bb94463331b6a968556b4d30ea1f57b25577e0d
Flags: needinfo?(felash)
Duplicate of this bug: 840085
Blocks: 875619
Flags: in-moztrap-
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.