Closed
Bug 1074026
Opened 10 years ago
Closed 10 years ago
webapp uninstall broken, Webapps:GetApkVersions throws
Categories
(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 35
People
(Reporter: Mook, Assigned: myk)
Details
Attachments
(2 files, 1 obsolete file)
4.48 KB,
text/plain
|
Details | |
2.02 KB,
patch
|
wesj
:
review+
|
Details | Diff | Splinter Review |
STR: - Install a few web apps from _way_ back - Attempt to uninstall them: - (Fennec menu) -> Tools -> Apps - Hold on an app icon - Select "Uninstall" from the popup that shows up Expected results: App is uninstalled. Actual results: Nothing visible. Additional information: ADB logcat says: 09-28 18:26:24.708: I/Gecko(6414): -- webapps.js uninstall http://example.com/manifest.webapp 09-28 18:26:24.708: W/InputMethodManagerService(816): Window already focused, ignoring focus gain of: com.android.internal.view.IInputMethodClient$Stub$Proxy@44d2bc18 attribute=null, token = android.os.BinderProxy@42fb4f28 09-28 18:26:24.718: D/audio_hw_primary(185): select_devices: out_snd_device(2: speaker) in_snd_device(0: ) 09-28 18:26:24.718: D/GeckoWebappManager(6414): uninstall: http://example.com/manifest.webapp 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): Exception handling message "Webapps:GetApkVersions": 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): java.lang.IllegalArgumentException: Property type mismatch 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.util.NativeJSObject.getStringArray(Native Method) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.webapp.EventListener.handleMessage(EventListener.java:83) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:163) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.GeckoAppShell.handleGeckoMessage(GeckoAppShell.java:2337) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:359) 09-28 18:26:24.728: E/GeckoWebappEventListener(6414): at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:176) Fennec 33.0 (beta channel), I can't seem to find the build ID in about:support for some reason. I might have removed the APK from the android app settings thing at some point? It's not showing up in that list.
Assignee | ||
Comment 1•10 years ago
|
||
(In reply to :Mook from comment #0) > Fennec 33.0 (beta channel), I can't seem to find the build ID in > about:support for some reason. > > I might have removed the APK from the android app settings thing at some > point? It's not showing up in that list. That's ok, it should still be possible to uninstall the app from about:apps, if it still appears there, per bug 1019054, where I implemented that for Firefox 33. So I'm not sure what's going on here. What happens if you reinstall the app and then try to uninstall it? And is there a particular app for which you're experiencing this problem? I see "example.com" manifest URLs in the log, and I'm assuming those are placeholders for the actual URLs.
Priority: -- → P1
Yep, I'm trying to uninstall things from about:apps I'm having trouble reinstalling the app (at least, I see no sign in the marketplace that I could; it just asks me to open it instead). It looks like launching them is also broken, huh. It just says: 10-01 19:10:50.598: D/GeckoWebappManager(24789): launchWebapp: http://littlealchemy.com/manifest.webapp Nothing else. Nothing in chrome://global/content/console.xul either, in case that matters. I have the same stack as above, for the following webapps (which are all that are installed in my Firefox): http://littlealchemy.com/manifest.webapp http://wordwars.clay.io/firefox.webapp http://aim-point-pool.tresensa.com/manifest.webapp http://taratatach.github.io/FFDoku/manifest.webapp http://battleships.arandomurl.com/manifest.webapp Installing a new webapp (http://around.lonelyvegan.com/manifest.webapp) and then uninstalling it from about:apps does work (at least, it got further to a prompt I hadn't seen); uninstalling it from the Android settings -> apps works too (and that removed it from Fennec, so I can't test uninstalling it again from Fennec)
Assignee | ||
Comment 3•10 years ago
|
||
Hmm, I'm having trouble reproducing on a build from the beta branch with a bunch of different web apps I'd installed a while ago. Perhaps not long enough ago? One possibility is that you're uninstalling apps that were installed before we made them APK-based, and which haven't been updated since. If so, then you may be able to work around the problem by tapping the Check for Updates item at the bottom of about:apps; going through the update process for those apps, which involves installing their APKs; and then uninstalling them. (That doesn't mean there isn't a problem here. Obviously you shouldn't have to do that in order to uninstall those apps. But if it works around the problem, then it's a clue about the issue.) Also, if you have the wherewithal, then, before doing that, send me a copy of the webapps.json file in your profile. It should be located at something like /data/data/org.mozilla.firefox_beta/files/mozilla/webapps/webapps.json.
Yep, updating and then uninstalling seems to work. Except that one app that doesn't seem to want to update (word wars). Webapps.json, pre-update, attached; I didn't see anything privacy-sensitive in it.
Assignee | ||
Comment 5•10 years ago
|
||
I'm having some trouble configuring a profile to confirm this, but I think I see the problem: WebappManager.uninstall sends app.apkPackageName to _getAPKVersions without first ensuring that it's a valid value (compare to _checkForUpdates, which filters out invalid values from the list it sends to _getAPKVersions). In which case this is the obvious fix. Requesting review, although I still need to test it.
Assignee: nobody → myk
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8501771 -
Flags: review?(wjohnston)
Assignee | ||
Comment 6•10 years ago
|
||
FWIW, this bug seems likely to be rare. It'll only happen when a user has apps from before APKs and never updates them (either because the daily update check fails or because the user ignores it). It might also depend on a failure in the code that synchronizes the registry with installed APKs (unsure).
Assignee | ||
Comment 7•10 years ago
|
||
It turns out that change was necessary but not sufficient. We also have to update the call to DOMApplicationRegistry.doUninstall, as DOMApplicationRegistry's various methods for uninstalling apps have changed. This patch is tested and verified to work.
Attachment #8501771 -
Attachment is obsolete: true
Attachment #8501771 -
Flags: review?(wjohnston)
Attachment #8501989 -
Flags: review?(wjohnston)
Comment 8•10 years ago
|
||
Comment on attachment 8501989 [details] [diff] [review] patch v2: also update registry method call Review of attachment 8501989 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/modules/WebappManager.jsm @@ +233,5 @@ > } > > // If the APK is installed, then _getAPKVersions will return a version > // for it, so we can use that function to determine its install status. > + if (app.apkPackageName && app.apkPackageName in (yield this._getAPKVersions([ app.apkPackageName ]))) { I'd probably leave these on two lines just to make it clearer these an async call in here, but up to you :) @@ +257,5 @@ > // The APK isn't installed, but remove the app from the registry anyway, > // to ensure the user can always remove an app from the registry (and thus > // about:apps) even if it's out of sync with installed APKs. > debug("APK not installed; proceeding directly to removal from registry"); > + DOMApplicationRegistry.uninstall(aData.manifestURL); Fabrice would yell at us for not having a test for this :(
Attachment #8501989 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #8) > I'd probably leave these on two lines just to make it clearer these an async > call in here, but up to you :) It's a bear, I agree! But I need to check if app.apkPackageName before calling (yield this._getAPKVersions([ app.apkPackageName ])), and if I separated the conditionals, then I'd have to nest conditional blocks and indent the existing block another level: if (app.apkPackageName) { let apkVersions = (yield this._getAPKVersions([ app.apkPackageName ])); if (app.apkPackageName in apkVersions) { … And I figured it would be better to retain the single block, despite the complexity of the conditional. > Fabrice would yell at us for not having a test for this :( So say we all! :-/ https://hg.mozilla.org/integration/fx-team/rev/8c0b4dfe1da4
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8c0b4dfe1da4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•