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

Status: RESOLVED FIXED

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

Description

[sync-1]

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:

Comment 1

[Julien Wajsberg [:julienw]]

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 ?

Comment 2

[Lukas Blakk [:lsblakk] use ?needinfo]

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?

Comment 3

[Jason Smith [:jsmith]]

Vicamo - Can you help address comment 1? Do we need to do this for 1.1?

Comment 4

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

does it have any certification impact? my understanding is yes but i'd like to have reporter to confirm

Comment 5

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

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

Comment 6

[Cheng-An, XIONG]

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.

Comment 7

[Vicamo Yang [:vicamo][: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.

Comment 8

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

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)

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Attachment: m-c patch

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

Attachment: m-c patch : v2

fix typo.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gaia PR

https://github.com/mozilla-b2g/gaia/pull/11690

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Attachment: b2g18 patch

Identical to attachment 793895 [details] [diff] [review], but with an additional override in b2g/app/b2g.js.  No Gaia PR is required.

Comment 13

[Julien Wajsberg [:julienw]]

changing title and component to reflect the current patch.

Should we file another bug to implement the "real" SMIL ? Joe what's your thought ?

Comment 14

[Steve Chung [:steveck]]

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.

Comment 15

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

(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?

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

(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 17

[Gene Lian (I already quit Mozilla)]

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?

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

(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.

Comment 19

[Vicamo Yang [:vicamo][:vyang]]

Attachment: b2g18 patch : v2

Don't override in b2g/.  Cherry-pick Gaia patch instead.

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/c8c5ffcf94a0
https://hg.mozilla.org/releases/mozilla-b2g18/rev/cb44fea7a502

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Sorry, it seems I went too much.  I should have waited b2g-inbound change gets merged in m-c.

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

Merged Gaia PRs:

1) master
https://github.com/mozilla-b2g/gaia/commit/68ef5d6bd6056c2769a42c5cf270cf1dcd41aefe
2) v1-train
https://github.com/mozilla-b2g/gaia/commit/3e10328ba03034feb6e7d3683d48445aac2d8d06
3) v1.1.0hd
https://github.com/mozilla-b2g/gaia/commit/14ae317bae9a57906b6c723fb0ab1641924be540

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c8c5ffcf94a0

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/cb44fea7a502

Comment 25

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: 3e10328ba03034feb6e7d3683d48445aac2d8d06
v1.1.0hd: 9beb8d0835e319c7aade3917ca3a2dc39cc60da2