1172486 - Add feature requirement for OpenMobile ACL

Status: RESOLVED FIXED

(Marketplace Graveyard :: General, defect, P1)

Description

[Mathieu Pillard [:mat]]

We are going to need a new feature requirement, to identify devices that are compatible with the Open Mobile App Compat Layer.

Bug 1157028 added the feature we need to detect, it's "acl.version". The value is irrelevant to us at the moment, it will be useful when different versions of the ACL appear. For now, just calling getFeature('acl.version') and checking that it's not undefined should be enough.

See https://www.lucidchart.com/documents/edit/1b634261-be39-4e5b-92c7-2712402a5041/0

Comment 1

[Mathieu Pillard [:mat]]

Note: NFC feature was added to zamboni in https://bugzilla.mozilla.org/show_bug.cgi?id=1150372 but AFAIK was never added to Fireplace. When adding Open Mobile ACL detection, we also need to add NFC, bumping the feature profile version to 8.

Comment 2

[Mathieu Pillard [:mat]]

Questions:
- What name and description should be given to the feature (to display with the checkbox in the Edit requirements page)
- Should we detect some API usage in the validator to tick the checkbox automatically ? If so, which API ?

Comment 3

[Didem Ersoz [:didem]]

Bill,

Are you able to help with Mathieu's questions in comment 2? If not, who should I ping?

Thanks,
Didem

Comment 4

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Mathieu Pillard [:mat] from comment #2)
> Questions:
> - What name and description should be given to the feature (to display with
> the checkbox in the Edit requirements page)

I would rather not show any text at all, if that's possible. Only one developer account will ever upload apps that have this requirement, and there will only be three of them this year. There will never be more than 30 such apps.

> - Should we detect some API usage in the validator to tick the checkbox
> automatically ? If so, which API ?

What if I told you there was going to be a special magic app manifest permission if and only if this is an OpenMobile app that has this requirement, would that help?

Comment 5

[Mathieu Pillard [:mat]]

We currently don't have the notion of hidden feature requirements, implementing that would take time...

Yes, a special manifest permission would help if there is one.

BTW, I am going to have to publish code mentioning the feature by name. Can I call this openmobile ? All the bugs were marked as confidential...

Comment 6

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Mathieu Pillard [:mat] from comment #5)
> We currently don't have the notion of hidden feature requirements,
> implementing that would take time...
> 
> Yes, a special manifest permission would help if there is one.
> 
> BTW, I am going to have to publish code mentioning the feature by name. Can
> I call this openmobile ? All the bugs were marked as confidential...

How about we call it something like "aclcompatible" since ACL is the name of the technology?

Comment 7

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Mathieu Pillard [:mat] from comment #5)
> We currently don't have the notion of hidden feature requirements,
> implementing that would take time...
> 
> Yes, a special manifest permission would help if there is one.

And yes there will be a special manifest permission, Fabrice is writing that code now.

Comment 8

[David Durst [:ddurst]]

One thing we discussed earlier is that, once aclcompatible is visible in some manifest, we have no way to constrain its use to this publisher.

Comment 9

[Mathieu Pillard [:mat]]

I noticed that at least https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0eaf53250fba already mentions Open Mobile, so I don't see a reason to hide it any longer ?

Calling the feature something cryptic like "ACL Compatible" seems counter-productive to me (it's going to be confusing even to us developers working on Marketplace code)

Comment 10

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Mathieu Pillard [:mat] from comment #9)
> I noticed that at least
> https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0eaf53250fba already
> mentions Open Mobile, so I don't see a reason to hide it any longer ?

In hindsight, that was not a good bug title. That should have been "add feature detection for ACL", since it adds the ability to call navigator.getFeature("acl.version").

> 
> Calling the feature something cryptic like "ACL Compatible" seems
> counter-productive to me (it's going to be confusing even to us developers
> working on Marketplace code)

OpenMobile is a company name. Application Compatibility Layer (ACL) is the name of the product we are integrating into Firefox OS. In the same way that adding support for Chrome plug-ins would not be called "Google compatibility", so also we would not call this current work "OpenMobile compatibility".

All the patches should be public next week.

Comment 11

[Mathieu Pillard [:mat]]

Just calling it "ACL" would be meaningless and confusing (it was certainly confusing to me when I started working on these bugs, I had no idea what "ACL" meant, had to... Google it). Even the full name, "Application Compatibility Layer", is too generic to understand, IMHO. How about calling it OpenMobile ACL ? or the real full name, which is "OpenMobile's Application Compatibility Layer (ACL)™" ?

Regarding patches: I have to submit pull requests, commit and push to our Marketplace repos starting today if we want to be ready for Friday. So my patches will have to be public today...

Comment 12

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Mathieu Pillard [:mat] from comment #11)

> Regarding patches: I have to submit pull requests, commit and push to our
> Marketplace repos starting today if we want to be ready for Friday. So my
> patches will have to be public today...

Yup, go for it. Thanks for your thoughtful comments. Are you proposing to name this requirement "openmobile-acl" or similar?

I really, really don't want developers to think that they can somehow submit their own APK's to marketplace, because they can't. Wouldn't a checkbox in the app submission flow labelled "openmobile ACL" invite them to think that they could do that?

Comment 13

[Mathieu Pillard [:mat]]

The requirement will be called "OpenMobile ACL", but I'm going to go the extra mile and hide the checkbox for developers - only Mozilla staff (and reviewers) will see it.

Comment 14

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Mathieu Pillard [:mat] from comment #13)
> The requirement will be called "OpenMobile ACL", but I'm going to go the
> extra mile and hide the checkbox for developers - only Mozilla staff (and
> reviewers) will see it.

Much appreciated, sir.

Comment 15

[[:fabrice] Fabrice Desré]

(In reply to Bill Walker [:bwalker] [@wfwalker] from comment #7)
> (In reply to Mathieu Pillard [:mat] from comment #5)
> > We currently don't have the notion of hidden feature requirements,
> > implementing that would take time...
> > 
> > Yes, a special manifest permission would help if there is one.
> 
> And yes there will be a special manifest permission, Fabrice is writing that
> code now.

right, this is happening in bug 1169472

Comment 16

[Mathieu Pillard [:mat]]

Featured added in https://github.com/mozilla/zamboni/commit/89f65be6297168197dabebb1d720005a64633a9c

The feature requirement is called "OpenMobile ACL", but is hidden from users without the Apps:Configure permission (most of mozilla staff users have this perm)

Validator work to follow.

Comment 17

[Mathieu Pillard [:mat]]

Permission added to validator in https://github.com/mozilla/app-validator/commit/0b6892b800302cbab558256191ad219a28daf990

Validator bumped in https://github.com/mozilla/zamboni/commit/924698c3376aca6402658f678a7b87b7cbbfc17a

Comment 18

[krupa raj[:krupa]]

I cannot fully verify this fix on dev/stage until https://bugzilla.mozilla.org/show_bug.cgi?id=1203706 is fixed.

Comment 19

[krupa raj[:krupa]]

Things tested that work:
1. Open mobile ACL feature is listed for mozilla users. See https://www.dropbox.com/s/ttjhgfgmgxw0iqn/Screenshot%202015-09-10%2014.41.36.png?dl=0
2. Open mobile ACL feature is not listed for non-mozilla users. See https://www.dropbox.com/s/rk0izglwiju4lep/Screenshot%202015-09-10%2014.43.31.png?dl=0
3. App with that flag selected is listed in the Consumer pages when adb shell setprop persist.acl.version ";P172R12" is set on the device
4. App is not listed for devices messing that feature.

Things that did not work:
4. After unchecking the value, the app is not listed on the device anymore. --> The app continued to be listed in New listing page. I think feature change doesn't force-trigger a refresh.

Pretty edge-casey. 

Mat, any thought?

Comment 20

[Mathieu Pillard [:mat]]

I'm pretty sure feature changes do trigger a re-index. It forces a save on the Webapp instance and therefore automatically triggers a re-index. What could have happened is caching on the client preventing you from seeing the changes ? There are 2 different layers of caching that could make it hard to see the changes right away (you need to wait 3 minutes + do a double reload to be extra-sure)