STR: * go to http://everlong.org/mozilla/ * install hosted app with app cache * install hosted app without app cache * everything seems ok * long press on one icon * uninstall one of them, this works correctly * try to uninstall the other, click on confirm Expected: * the icon should disappear Actual : * the icon doesn't disappear Then : * reboot the phone => the icon has disappeared * try to install the app again => it never installs There is no meaningful log. I think this happens whether there is an appcache or not. And it happens only if I install both apps, not if I install only one of them.

:julienw if you install all the apps from the same domain, you will keep just the latest app, that means effectively you will be able to have just ONE app per ORIGIN. :(

francisco> so we must forbid to install it.. the behaviour is very confusing at the moment imho. And it works for packaged app !

Rare use case for v1. This definitely does not block.

It works for packaged apps because we create a new fake origin when we install them. For hosted apps, we are unfortunately stuck with the "one app per origin" rule for v1. At the platform level, we consider that as a re-install of the same app. I'm not sure what the right fix it here to make it not confusing for the user, but gaia could check when we do the webapps-ask-install if we already have an app installed from the same origin but with a different manifest URL and reject installs?

To stay consistent with other behaviour, I think we should have a downloaderror event with a specific name sent by the platform. We should not present a dialog because the user can't do much, rather we'll output to the console the error name, as we already do for the other non-user-fixable errors. I'd like the platform to check this because it smells better. :-) Sounds good ?

Disagree on the blocking call. Multiple apps per origin is not a v1 use case. This should be a minus.

Marketplace technically enforces this restriction as well.

(In reply to Jason Smith [:jsmith] from comment #7) > Marketplace technically enforces this restriction as well. As a result of the enforcement, this is why we haven't blocked on this issue on the equivalent bug for the desktop and android implementations.

(In reply to Julien Wajsberg [:julienw] from comment #5) > To stay consistent with other behaviour, I think we should have a > downloaderror event with a specific name sent by the platform. We should not > present a dialog because the user can't do much, rather we'll output to the > console the error name, as we already do for the other non-user-fixable > errors. > > I'd like the platform to check this because it smells better. :-) > > Sounds good ? Platform will not fix this bug for basecamp. We are not supporting this use case for v1.

David, Jason, the blocker was about properly forbidding this behaviour. Renominating, because we need to properly support not supporting this ;)

I'm pretty sure this is not a rare use case. Most developers have access to one server with one domain, and they'll try to install multiple apps on the same origin. If this fails randomly, this is very bad, so we should handle correctly this and report a correct error. Please block on this.

(In reply to Julien Wajsberg [:julienw] from comment #10) > David, Jason, the blocker was about properly forbidding this behaviour. > > Renominating, because we need to properly support not supporting this ;) Oh. Well, in that case, I agree. Let me flip the title to reflect this.

(In reply to Julien Wajsberg [:julienw] from comment #11) > I'm pretty sure this is not a rare use case. Most developers have access to > one server with one domain, and they'll try to install multiple apps on the > same origin. If this fails randomly, this is very bad, so we should handle > correctly this and report a correct error. > > Please block on this. Right. Disabling is fine for v1. I understand the nature of needing this. I believe support for this should definitely be a v2 feature though.

We won't add a downloaderror event since this is not an error. If you check the webapps registry, only one app is installed, even if gaia added two icons.

Fabrice, I don't understand why this is not an error. In the webapps registry, is there the old one or the new one ? I really believe it should be the old one, and an error should be sent to prevent adding the new one. Any other behavior seems confusing.

As I said in comment 4, installing again from the same origin is considered as a reinstall. You get the new app in the registry. That's the only way we can let reinstall work. The fact that we support only one app per origin is the root issue, but we won't fix that for v1 unfortunately.

Ok, I get your point. However I don't get how reinstall could work for packaged app. Is reinstall a feature ? In which case could this be useful ? I think that we didn't pay a lot of attention in Gaia to the reinstallation use case anyway. If I want to install the same hosted app, the dialog just asks to install the app. Same for packaged app, but for packaged app there is actually another copy. Really this is not consistent, I'm asking josh for a comment here.

(In reply to Julien Wajsberg [:julienw] from comment #17) > Ok, I get your point. > > However I don't get how reinstall could work for packaged app. Is reinstall > a feature ? In which case could this be useful ? Sadly, reinstall is out of scope for v1. Marketplace UX I recall determined this as a very low priority user flow, as we're expecting installs, updates, and uninstalls. Reinstall didn't seem likely if the other three exist for v1. > > I think that we didn't pay a lot of attention in Gaia to the reinstallation > use case anyway. If I want to install the same hosted app, the dialog just > asks to install the app. Same for packaged app, but for packaged app there > is actually another copy. Sounds like the same situation that happened on desktop and android. Our resolution with those was that we'd only block on reinstall flows on "castrophic" behavior that results from a reinstall. But technically marketplace has a hard control over the one app per origin rule, so the concern of satisfying this workflow is quite low priority at the moment. > > Really this is not consistent, I'm asking josh for a comment here. I'll wait for UX to comment, but I'm inclined to wont fix this bug as it stands right now based on our past precedents we've followed with other platforms.

I'm ok with not supporting reinstall, but I do want to properly support not supporting, I mean we should explicitely forbid this and not put the platform in a strange state (here: an icon is left on the homescreen until we reboot the phone). Let's remember the marketplace is not the only way to install apps, this is our commitment and our value here, and people _will_ try to install different apps from the same website.

Relevant prior discussion is here: https://bugzilla.mozilla.org/show_bug.cgi?id=790558#c14 My specs also discussed this: > Removed: “Check for Duplicates” step, per Jonas’s Oct 1 comments in bug 790558. > He explained that reinstalls will be low-risk edge cases. Marketplace will > have mechanisms for preventing, and in event they do happen (if user reinstalls > an existing app from a non- Marketplace source, for example), reinstall will > simply re-download app code, leaving user data and app state untouched. App Install specs: https://www.dropbox.com/sh/b0kyykhzcfkpm8b/ReFTX_E72X I've only had time to skim this thread, so let me know if I've missed anything.

Josh> I think it works correctly if the user installs the exact same application (ie: same manifest URL). However, the problem here is if the user installs an application with a different manifest URL but with the same origin. Currently, the platform handles this as a reinstall for hosted apps (same origin), but as a new install for packaged app (because it generates a new origin). But Homescreen handles this as a new install for both. This leads to an inconsistency between the platform and Homescreen for hosted apps, the problem pointed by this bug. I think this will definitely happen, as I suspect hosted apps will certainly be used by out-of-marketplace apps.

Hi Julien, I apologise for the tardy response on this item. You're more familiar with the specifics of the issue here. What would you recommend from a UX standpoint? Is this something we can address with an error message to the user, for example?

Well, it's bb- :) If we'd do something at this point, we should do whatever is the simplest to do: the homescreen should handle hosted apps with same origin as a reinstall, ie removing the older app.

nominating again now that we're post-basecamp.

I sure hope than in 19+ we'll support the multiple app from the same origin use case :-)

Blocking on ensuring that we throw an error as appropriate here.

So apparently what happens here on the platform side is that when you install a second app from the same origin. However it doesn't appear that gaia is handling the events that we fire as that is done. If I understood Fabrice correctly we fire a install event on the existing app, and Gaia isn't handling that properly.

Jonas, that's basically what I said in comment 23. However what I dislike with that is that : * for packaged app, everything works fine because we generate a new origin (app://xxx) * for hosted app, the new install overwrites the previous one. The user has no mean to know that. That's why I think we should prevent this instead, or at least display a warning. I'm perfectly fine with all that happening in Gaia. Josh, what do you think ?

(In reply to Julien Wajsberg [:julienw] from comment #28) > That's why I think we should prevent this instead, or at least display a > warning. > > Josh, what do you think ? I agree that preventing this situation or at least warning the user would be preferable to the current status quo.

This does not impact the marketplace, will be immediately obvious (and fixable) if a developer makes this mistake, and doesn't leave the user in an unrecoverable state. not blocking-b2g:-.

Alex, I agree with the fact that this not blocks, but I disagree that this will be immediately obvious. And I disagree also that this is easily fixable... for the user, because it actually removes the old app and takes the new app. So he will have to know which app was removed, which is clearly not easy because he wasn't warned at all and he has no mean to know this. Let's take a concrete example: apps on github. Same developer apps on github will share the same origin : https://<username>.github.com. I anticipate that this will be _very_ frequent. Problem here is that if you install only _one_ app from this origin, the problem is not evident. The problem is evident if you install _two_ apps. I agree that the developer will most probably try to install its apps first and then it will exhibit this behavior. So I'm not changing the triage but I ask you to consider this problem as soon as we begin to work on v-next.

Discussion on this bug makes me think it's time to consider multiple apps per origin in v2. I'll talk to Bill about this a bit more.

Actually, i think the situation is quite a bit worse here. The user can easily end up nuking an application without realizing since we'll display an install dialog, but we're actually replacing one app for another. I think the simplest solution here is to simply raise an error if someone tries to install an app for a same-origin-but-different-manifestURL than an already installed app. That also gives us a better migration path for once we support multiple apps per origin.

(In reply to Jonas Sicking (:sicking) from comment #33) > Actually, i think the situation is quite a bit worse here. The user can > easily end up nuking an application without realizing since we'll display an > install dialog, but we're actually replacing one app for another. Technically that problem happens on all platforms right now, not just b2g. > > I think the simplest solution here is to simply raise an error if someone > tries to install an app for a same-origin-but-different-manifestURL than an > already installed app. > > That also gives us a better migration path for once we support multiple apps > per origin. I agree, but if we're going to fix this bug, I'm arguing we fix this in gecko, not gaia. The same problem you are talking about is a cross-platform platform, not a gaia problem.

well, I was currently carefully crafting a patch in Gaia ;) It's almost ready. Should I stop now ? :)

Blocking on simply raising an error here. We're not going to implement multiple apps per origin in v1. So we should simply return an error if you try to install a same-origin-different-manifestURL app.

PR: https://github.com/mozilla-b2g/gaia/pull/7684 This is my current work. I believe this is complete but I didn't test it on the device, yet I have created unit tests for this. I don't ask for review unless we decide this could land in Gaia for v1. This patch prevents installing an app with the same origin than another app. This displays an error "install failed" to the user and outputs a more useful log to the console. --- apps/system/js/app_install_manager.js | 19 ++++++++- apps/system/js/applications.js | 13 ++++++ apps/system/test/unit/app_install_manager_test.js | 45 ++++++++++++++++++++- apps/system/test/unit/applications_test.js | 35 ++++++++++++++++ apps/system/test/unit/mock_applications.js | 15 +++++++ apps/system/test/unit/mock_chrome_event.js | 5 ++- 6 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 apps/system/test/unit/applications_test.js

This patch prevents reinstalling : - hosted apps from the same origin - packaged apps with the same manifest URL I added the packaged app check to keep a consistent user experience. Once we have multiple apps per origin, we should just use the manifest URL in both cases. The new error returned is REINSTALL_FORBIDDEN

Why would we prevent reinstall all together? Couldn't we just allow reinstall if the manifest URL of the current app that is installed matches the manifest URL provided to install the app? If they don't match, then throw the error.

We could do that, if that doesn't confuse gaia too much.

If reinstalling an app causes gaia confusion then I don't think we should mess with it. Reinstalling an app is a weird edgecase anyway and most store UIs change the "install" button to a "launch" button when they detect that an app is installed, so in most cases it's not something we'll hit.

Comment on attachment 703984 [details] [diff] [review] Gecko patch Review of attachment 703984 [details] [diff] [review]: ----------------------------------------------------------------- works for me I tried different use cases: packaged, hosted, try to install during an install, try to install during an update couldn't try to install during an appcache update (because of another bug) but this seems quite solid and gaia is happy with this. r=me with the nits ::: dom/apps/src/Webapps.jsm @@ +1526,5 @@ > }.bind(this); > > + // Disallow reinstalls from the same origin for now. > + // See https://bugzilla.mozilla.org/show_bug.cgi?id=821288 > + if (this._appId(app.origin) != null) { nit: for comparing with null I'd use !== otherwise you're comparing also to undefined (is that the case ?) @@ +1597,5 @@ > }.bind(this); > > + // Disallow reinstalls from the same manifest URL for now. > + // See https://bugzilla.mozilla.org/show_bug.cgi?id=821288 > + if (this.getAppLocalIdByManifestURL(app.manifestURL) != nit: I don't know well all this but I'd use !== just to be on the safe side.

(In reply to Jonas Sicking (:sicking) from comment #41) > If reinstalling an app causes gaia confusion then I don't think we should > mess with it. Reinstalling an app is a weird edgecase anyway and most store > UIs change the "install" button to a "launch" button when they detect that > an app is installed, so in most cases it's not something we'll hit. Yeah, true. Rethinking about it, let's actually go with turning off reinstall then. Reinstall has been buggy in the past if we switch things around (new zip package for example), so it's probably safe to go with the "turn off" strategy as shown by this patch.

Comment on attachment 703984 [details] [diff] [review] Gecko patch BTW we get nothing displayed in Gaia for these errors that happen before the app is installed in the registry. Right now, this should be handled by the marketplace or the app developer.

(In reply to Julien Wajsberg [:julienw] from comment #37) > Created attachment 703918 [details] [diff] [review] > gaia patch Is this ready for review now?

Comment on attachment 703918 [details] [diff] [review] gaia patch we'll do this in gecko and not in gaia.

Verified on 1/22 with one followup needed. See bug 833659 for a followup.

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.