Build: 10/17/2013 Master Buri STR 1. Setup your build according to https://wiki.mozilla.org/B2G/QA/Customizations#Build_Setup with a AT&T SIM included 2. Complete the FTE & enable remote debugging 3. Analyze webapps.json Expected Poppit should have an origin of app://poppit. Actual Poppit has a default origin - app://{1a8bc3f8-f05c-4ce2-a913-3f57423edcb7}

This apparently works fine for the marketplace packaged app, which makes me think this is related to how we preinstall custom packaged apps from the distribution directory.

gecko part, making sure we don't override the origin from the metadata for operator variant customizations.

Not sure what the original code was trying to do tbh...

There is a doc about it: https://developer.mozilla.org/en-US/Apps/Developing/Manifest#origin. The doc says "You do not need this field for hosted apps, which already have an origin (the website that they are served from)". The original code doesn't create the origin for the hosted app whose the manifest.webapp doesn't contain the origin field. But it creates the origin for the hosted app even if it has a origin field. Maybe, we can use this bug to fix the creation of hosted app or create another one to deal with it.

Oh! Sorry. There is a typo in comment4, the correct one is: The original code doesn't create the origin for the hosted app whose the manifest.webapp doesn't contain the origin field. But it creates the origin for the "packaged" app even if it has a origin field.

I'm not sure what you mean. Are you saying that the current code is correct?

No, I say the current code is incorrect. You patch is correct. But there one another issue in the current code: we should generate "origin" field for hosted app, but we don't have it in current code.

Comment on attachment 818626 [details] [diff] [review] gecko patch ignore the dump()...

Comment on attachment 818626 [details] [diff] [review] gecko patch Review of attachment 818626 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/apps/src/Webapps.jsm @@ +2192,5 @@ > app.id = id; > > let manifestName = "manifest.webapp"; > if (aData.isPackage) { > // Override the origin with the correct id. Could you add a comment explaining the reason for the condition, please?

Comment on attachment 818628 [details] [diff] [review] gaia patch looks good to me. Albert, could you double check everything is good with this commit for firefoxos-gaia-spain/sample/variant.json?

Comment on attachment 818628 [details] [diff] [review] gaia patch As I see, with this patch the origin is not mandatory. If it doesn't exist, gecko will assign the id as a install origin, so we need to remove the line launching an exception if no origin found at build/variant.py:72 if not origin: raise Exception("installOrigin required for app '" + app_id + "' in local_apps.json configuration file")

I'm not sure I understand the patch. The metadata origin should not be used to set the packaged apps origin, since it's being used to supply the missing data to make it behave like mozApps.installPackage (or mozApps.install) was called. The packaged app origin should be set either to an app://randomid or to app:// + manifest.origin if and only if the app is signed and the app is privileged [1]. If we desire all local installation (which are what single variant apps are) to be allowed to specify an origin, then it should suffice changing line 2868 to read: if ((isSigned || isLocalFileInstall) && shouldn't it? In any case, Poppit does NOT have an origin specified on the manifest ([2]) https://marketplace.firefox.com/app/d37cb5ed-525a-408e-a7cf-ec848064e041/manifest.webapp so it should *not* have a special manifest set. The original behavior, then, was the correct one. Operator apps are automatically installed apps, but they're normal apps for all practical senses and so they should behave as normal apps do, [1] https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#2868

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #12) > I'm not sure I understand the patch. The metadata origin should not be used > to set the packaged apps origin, since it's being used to supply the missing > data to make it behave like mozApps.installPackage (or mozApps.install) was > called. The packaged app origin should be set either to an app://randomid or > to app:// + manifest.origin if and only if the app is signed and the app is > privileged [1]. I don't think that matches old behavior nor behavior that marketplace already relies on. Old behavior allowed a preinstalled packaged app to specify an origin on preinstall. > > If we desire all local installation (which are what single variant apps are) > to be allowed to specify an origin, then it should suffice changing line > 2868 to read: > > if ((isSigned || isLocalFileInstall) && Does this work right now? We should confirm if the local operator apps use case allows this. > > shouldn't it? > > In any case, Poppit does NOT have an origin specified on the manifest ([2]) > > https://marketplace.firefox.com/app/d37cb5ed-525a-408e-a7cf-ec848064e041/ > manifest.webapp > > so it should *not* have a special manifest set. I don't think we've followed those rules historically. Preinstalled packaged apps have been allowed since 1.01 to specify their origin. > > The original behavior, then, was the correct one. Operator apps are > automatically installed apps, but they're normal apps for all practical > senses and so they should behave as normal apps do, Note - that won't be compatible with past FxOS revisions then.

(In reply to Jason Smith [:jsmith] from comment #13) > (In reply to Antonio Manuel Amaya Calvo (:amac) from comment #12) > > I'm not sure I understand the patch. The metadata origin should not be used > > to set the packaged apps origin, since it's being used to supply the missing > > data to make it behave like mozApps.installPackage (or mozApps.install) was > > called. The packaged app origin should be set either to an app://randomid or > > to app:// + manifest.origin if and only if the app is signed and the app is > > privileged [1]. > > I don't think that matches old behavior nor behavior that marketplace > already relies on. Old behavior allowed a preinstalled packaged app to > specify an origin on preinstall. That was so because the preinstalled apps don't pass through the normal installation. And that behavior wasn't correct. Because the same app (say poppit) would have an app://knownorigin url (opposed to an app://uuid url) only if it was preinstalled. So if I got two phones, one with it preinstalled and other that downloaded it from the marketplace, they would behave differently. Even worse, should Poppit developer add an origin to the manifest (which is the correct place to add it) later on, if that manifest origin URI was different from a the preinstalled one then the preinstalled ones wouldn't be able to update anymore. > > > > > If we desire all local installation (which are what single variant apps are) > > to be allowed to specify an origin, then it should suffice changing line > > 2868 to read: > > > > if ((isSigned || isLocalFileInstall) && > > Does this work right now? We should confirm if the local operator apps use > case allows this. Actually this should not be necessary if the operator apps keep the signature (depending on the solution taken for bug 926219). But if we want to allow specifying an origin to all operator apps (independently of if they come from a signed origin or not) this should be the place to change it to make it work consistently on all places. > > > > > shouldn't it? > > > > In any case, Poppit does NOT have an origin specified on the manifest ([2]) > > > > https://marketplace.firefox.com/app/d37cb5ed-525a-408e-a7cf-ec848064e041/ > > manifest.webapp > > > > so it should *not* have a special manifest set. > > I don't think we've followed those rules historically. Preinstalled packaged > apps have been allowed since 1.01 to specify their origin. And it's an error to do so, see my previous comment why. > > > > > The original behavior, then, was the correct one. Operator apps are > > automatically installed apps, but they're normal apps for all practical > > senses and so they should behave as normal apps do, > > Note - that won't be compatible with past FxOS revisions then. That behavior was just for build time and it can (and will) cause problems down the line.,

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #14) > (In reply to Jason Smith [:jsmith] from comment #13) > > (In reply to Antonio Manuel Amaya Calvo (:amac) from comment #12) > > > I'm not sure I understand the patch. The metadata origin should not be used > > > to set the packaged apps origin, since it's being used to supply the missing > > > data to make it behave like mozApps.installPackage (or mozApps.install) was > > > called. The packaged app origin should be set either to an app://randomid or > > > to app:// + manifest.origin if and only if the app is signed and the app is > > > privileged [1]. > > > > I don't think that matches old behavior nor behavior that marketplace > > already relies on. Old behavior allowed a preinstalled packaged app to > > specify an origin on preinstall. > > That was so because the preinstalled apps don't pass through the normal > installation. And that behavior wasn't correct. Because the same app (say > poppit) would have an app://knownorigin url (opposed to an app://uuid url) > only if it was preinstalled. So if I got two phones, one with it > preinstalled and other that downloaded it from the marketplace, they would > behave differently. Even worse, should Poppit developer add an origin to the > manifest (which is the correct place to add it) later on, if that manifest > origin URI was different from a the preinstalled one then the preinstalled > ones wouldn't be able to update anymore. An app developer I don't think is allowed to change an origin on marketplace - I think we locked that down during the 1.1 timeframe. In both cases actually, the app would fail to update anyways, as you can't change the origin on an update. > > > In any case, Poppit does NOT have an origin specified on the manifest ([2]) > > > > > > https://marketplace.firefox.com/app/d37cb5ed-525a-408e-a7cf-ec848064e041/ > > > manifest.webapp > > > > > > so it should *not* have a special manifest set. > > > > I don't think we've followed those rules historically. Preinstalled packaged > > apps have been allowed since 1.01 to specify their origin. > > And it's an error to do so, see my previous comment why. Fair enough. We'll need to get marketplace to update it's implementation to include an origin then. > > > > > > > > > The original behavior, then, was the correct one. Operator apps are > > > automatically installed apps, but they're normal apps for all practical > > > senses and so they should behave as normal apps do, > > > > Note - that won't be compatible with past FxOS revisions then. > > That behavior was just for build time and it can (and will) cause problems > down the line., Well, we'll have a problem I think either direction, but we're going to need to make the change you are suggesting anyways. Let's do this - I'll close this out as invalid, but file a new bug to get marketplace to update it's implementation to include an origin. Then, we'll need to disable recognizing the origin property in metadata.json in external apps.

Filed bug 929600 for fixing the issue on the marketplace side.

Also filed bug 929602 for removing origin setting in the old metadata.json format.