Closed Bug 905087 Opened 9 years ago Closed 9 years ago

[MMS] decrease the supported version in the X-Mms-Version header

Categories

(Core :: DOM: Device Interfaces, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla26
blocking-b2g leo+
Tracking Status
firefox24 --- wontfix
firefox25 --- wontfix
firefox26 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: sync-1, Assigned: vicamo)

References

Details

Attachments

(3 files, 2 obsolete files)

AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.184
 Firefox os  v1.1
 Mozilla build ID:20130806071254
 DEFECT DESCRIPTION:
 MMS messages are not presented as defined by the SMIL part of a received MMS
 
  REPRODUCING PROCEDURES:
 Description: Altough the device states support of MMS version 1.3 in its X-Mms-Version header MMS message are not presented as defined by the corresponding SMIL part of a received message (e.g. region layout - text above pictur or below).  A dedicated option to basically "play" the MMS as on many other terminals is not available.
 
 Additionally there are no options to configure/change the layout/ presentation while composing a MMS (e.g. w.r.t. slide timing, to insert a new slide).
 
 
 And X-Mms-Version should not be 1.3, but 1.1.
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 100%
 
  For FT PR, Please list reference mobile's behavior:
blocking-b2g: --- → koi?
blocking-b2g: koi? → leo?
Vicamo, Gene, do you think we should decrease the mms version ?

Cheng-an, we actually generate SMIL while composing (even if th user doesn't have much control on it), would decreasing the MMS version make this fail ?
Dietrich can you weigh in here on the risks/rewards of decreasing and whether this is something we should be testing/verifying for v1.1 or wait until v1.2?
Flags: needinfo?(dietrich)
Vicamo - Can you help address comment 1? Do we need to do this for 1.1?
Flags: needinfo?(vyang)
does it have any certification impact? my understanding is yes but i'd like to have reporter to confirm
Flags: needinfo?(sync-1)
vicamo, if this is blocking cert, how about the risk of changing this? is it simply a change of text from 1.3 to 1.1? Thanks
yes it impacts certifications.

1. if follow v1.3, then those v1.3 MANDATORY features should be supported, such as issue reported in this bug. if not support, then need apply for waiver for certification.

2. if follow v1.1, then PTCRB lab test will test v1.1 MANDATORY features according to v1.1 SPEC.

X-Mms-Version in UA should be consistent with actual MMS feature level.
Flags: needinfo?(sync-1)
Assignee: nobody → vyang
Sure, I will add an integer preference "dom.mms.version" with initial value '19'(version 1.3).  For m-c, a patch for Gecko and a GitHub pull request for Gaia will be landed; for b2g18 branch, a simple patch that overwrites MMS_VERSION will be landed for Gecko only.
Flags: needinfo?(vyang)
leo+ base on triage with partner for v1.1 release for shira partner and per discussion with Vicamo on the changes required (which is just one line for v1.1)
blocking-b2g: leo? → leo+
Flags: needinfo?(dietrich)
Attached patch m-c patch (obsolete) — Splinter Review
Attachment #793866 - Flags: review?(gene.lian)
Attached patch m-c patch : v2Splinter Review
fix typo.
Attachment #793866 - Attachment is obsolete: true
Attachment #793866 - Flags: review?(gene.lian)
Attachment #793895 - Flags: review?(gene.lian)
Attached patch b2g18 patch (obsolete) — Splinter Review
Identical to attachment 793895 [details] [diff] [review], but with an additional override in b2g/app/b2g.js.  No Gaia PR is required.
changing title and component to reflect the current patch.

Should we file another bug to implement the "real" SMIL ? Joe what's your thought ?
Component: Gaia::SMS → DOM: Device Interfaces
Product: Boot2Gecko → Core
Summary: [Buri][Shira-51064] MMS messages are not presented as defined by the SMIL part of a received MMS → [MMS] decrease the supported version in the X-Mms-Version header
Comment on attachment 793898 [details] [diff] [review]
Gaia PR

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

Since it's temporary override in the mms version, it should be fine to add this in preference.js directly for now. It would be better if you tag TODO in comment that we need to remove this override after we support vCard/vCalednar.
Attachment #793898 - Flags: review?(schung) → review+
(In reply to Julien Wajsberg [:julienw] from comment #13)
> changing title and component to reflect the current patch.
> 
> Should we file another bug to implement the "real" SMIL ? Joe what's your
> thought ?

i think Vicamo is going to do different implementation for m-c and v1.1 (per comment 7) so this bug should cover the "real" SMIL case?
(In reply to Joe Cheng [:jcheng] from comment #15)
> i think Vicamo is going to do different implementation for m-c and v1.1 (per
> comment 7)

After talked with Steve, having different impl. here doesn't mean a lot because we can simply cherry-pick Gaia PR to v1-train branch.  However, we still need a separate b2g18 patch for Gecko because the paths of patching files changed.

> so this bug should cover the "real" SMIL case?

We have neither "ics_landscape" nor page timing in MMS AICS, so I don't even think those extra SMIL options are in our consideration.
Comment on attachment 793895 [details] [diff] [review]
m-c patch : v2

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

Looks great! Thanks!

::: dom/mobilemessage/src/gonk/MmsPduHelper.jsm
@@ +13,5 @@
>  
>  let DEBUG; // set to true to see debug messages
>  
> +this.MMS_VERSION = (function () {
> +  Cu.import("resource://gre/modules/Services.jsm");

Why not just importing this in the global space?
Attachment #793895 - Flags: review?(gene.lian) → review+
(In reply to Gene Lian [:gene] from comment #17)
> Why not just importing this in the global space?

Because we only need |Services.jsm| in this function, at least for now.
Attached patch b2g18 patch : v2Splinter Review
Don't override in b2g/.  Cherry-pick Gaia patch instead.
Attachment #793905 - Attachment is obsolete: true
Sorry, it seems I went too much.  I should have waited b2g-inbound change gets merged in m-c.
https://hg.mozilla.org/mozilla-central/rev/c8c5ffcf94a0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
v1.1.0hd: 3e10328ba03034feb6e7d3683d48445aac2d8d06
v1.1.0hd: 9beb8d0835e319c7aade3917ca3a2dc39cc60da2
You need to log in before you can comment on or make changes to this bug.