877932 - checkInstalled should check if the app is locally installed

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Shuhao]

Steps to reproduce:
 
 1. Install a webapp
 2. Uninstall it by finding its executable and call it with -remove
 3. Reinstall the webapp < here it will fail and give you different error messages depending on the platform.

This has been tested on linux and mac.

Workaround:

If you only have one web app, you could uninstall via the browser console with the script

var r = navigator.mozApps.mgmt.getAll(); 
r.onsuccess = function() { navigator.mozApps.mgmt.uninstall(r.result[0]); }

Might have to play with that though.

Comment 1

[Marco Castelluccio [:marco]]

(In reply to Shuhao Wu [:Pwnna] from comment #0)
> Steps to reproduce:

I can't reproduce. Is there a particular application that shows this behaviour?

Comment 2

[Marco Castelluccio [:marco]]

(In reply to Shuhao Wu [:Pwnna] from comment #0)
> This has been tested on linux and mac.

(Note that on Mac there isn't a "remove" cmdline option)

Comment 3

[Shuhao]

Does not work still.

Installed app from http://osumo.paas.allizom.org/install. Uninstalled at:

[~/.offlinemozillasupport-9222bf00162f8a5a4be38ead9851192d]$ ./webapprt-stub -remove

Notification says it is removed. The icon goes away from system menu. Go back to the site to find the install button disappear (that button disappear if it detects user already installed the app). You can also find the app in navigator.mozApps.

Comment 4

[Marco Castelluccio [:marco]]

The app is actually uninstalled cleanly, the problem here is in checkInstalled that doesn't check if the app is locally installed.

Comment 5

[Marco Castelluccio [:marco]]

Attachment: checkInstalled_isLaunchable

Comment 6

[[:fabrice] Fabrice Desré]

Comment on attachment 785493 [details] [diff] [review]
checkInstalled_isLaunchable

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

Does that mean that the main registry is not updated? Doesn't that prevent reinstalling the same app?

Comment 7

[Marco Castelluccio [:marco]]

(In reply to Fabrice Desré [:fabrice] from comment #6)
> Comment on attachment 785493 [details] [diff] [review]
> checkInstalled_isLaunchable
> 
> Review of attachment 785493 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Does that mean that the main registry is not updated? Doesn't that prevent
> reinstalling the same app?

Yes, the main registry isn't updated. It's always the problem that the registry isn't in sync with applications installed, so we need to check if apps are actually installed with isLaunchable.

Comment 8

[[:fabrice] Fabrice Desré]

You did not answer my second question... Also, I would really prefer us to fix the registry update properly.

Comment 9

[Marco Castelluccio [:marco]]

(In reply to Fabrice Desré [:fabrice] from comment #8)
> You did not answer my second question...

Sorry, I should've been more clear. We always check if the app is locally installed, so an app can be installed if it is in the registry but isn't locally installed.

> Also, I would really prefer us to fix the registry update properly.

I agree, I'd like to have the apps in sync too, but it's a long term task and probably not feasible on all the platforms (on Mac we'll probably need to call something similar to _isLaunchable anyway).

I think, as all the other mozApps API calls are working correctly, for consistency we should make also checkInstalled work correctly.

Comment 10

[Marco Castelluccio [:marco]]

Attachment: checkInstalled_isLaunchable

Updated commit message.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/7b018235edc0

Could/should this have a test?

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Backed out for test_app_uninstall.html timeouts. Still wondering if this needs a test too.
https://hg.mozilla.org/integration/b2g-inbound/rev/1ec1d8b042e4

https://tbpl.mozilla.org/php/getParsedLog.php?id=27825182&tree=B2g-Inbound

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Nevermind, I was being dumb. This wasn't the bug at fault. Re-landed.
https://hg.mozilla.org/integration/b2g-inbound/rev/2981946d58f4

Comment 14

[Marco Castelluccio [:marco]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #11)
> https://hg.mozilla.org/integration/b2g-inbound/rev/7b018235edc0
> 
> Could/should this have a test?

We could add a small test to test_install_app.xul. I've written the patch, I'm waiting for try results.
Should I open a new bug for that? Or is it better to land the test here too?

Comment 15

[Marco Castelluccio [:marco]]

Opened bug 916355.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2981946d58f4