Last Comment Bug 774142 - Webapp uninstallation on Linux through mozApps uninstall function
: Webapp uninstallation on Linux through mozApps uninstall function
Status: RESOLVED FIXED
: productwanted
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: Trunk
: All Linux
: -- normal
: Firefox 17
Assigned To: Marco Castelluccio [:marco]
: Jason Smith [:jsmith]
Mentors:
Depends on: 761806
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-15 18:06 PDT by Marco Castelluccio [:marco]
Modified: 2016-02-04 15:00 PST (History)
3 users (show)
See Also:
QA Whiteboard: [qa-]
Iteration: ---
Points: ---


Attachments
Patch (1.59 KB, patch)
2012-07-15 18:06 PDT, Marco Castelluccio [:marco]
felipc: feedback+
Details | Diff | Splinter Review
WIP (3.41 KB, patch)
2012-08-05 18:26 PDT, Marco Castelluccio [:marco]
no flags Details | Diff | Splinter Review
WIP (3.46 KB, patch)
2012-08-05 18:33 PDT, Marco Castelluccio [:marco]
felipc: feedback+
Details | Diff | Splinter Review
Patch (5.60 KB, patch)
2012-08-07 17:46 PDT, Marco Castelluccio [:marco]
felipc: review+
lukasblakk+bugs: approval‑mozilla‑aurora-
Details | Diff | Splinter Review

Description Marco Castelluccio [:marco] 2012-07-15 18:06:55 PDT
Created attachment 642456 [details] [diff] [review]
Patch
Comment 1 :Felipe Gomes (needinfo me!) 2012-07-16 16:54:53 PDT
Comment on attachment 642456 [details] [diff] [review]
Patch

thanks Marco. WebappOSUtils.jsm is already in the tree so we should implement this there directly. Also, turns out XP_UNIX is also defined both for Android and B2G, so I'm trying to come up with a proper solution on how to handle this. Ideas appreaciated :)
Comment 2 Marco Castelluccio [:marco] 2012-08-05 18:26:11 PDT
Created attachment 649159 [details] [diff] [review]
WIP

What do you think about the approach about the XP_UNIX problem? Is it too chaotic?
Comment 3 Marco Castelluccio [:marco] 2012-08-05 18:33:32 PDT
Created attachment 649161 [details] [diff] [review]
WIP

Oops, forgot to qref.
Comment 4 :Felipe Gomes (needinfo me!) 2012-08-06 17:39:44 PDT
Comment on attachment 649161 [details] [diff] [review]
WIP

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

So instead let's do the better aprroach that we discussed about just dispatching a webapps-uninstall notification in Webapps.jsm, and then webappsUI can catch that notification and call WebappOSUtils.uninstall.  This will ensure this code is only used in desktop.

Moving forward we might split this file into one per OS (still thinking about that), but let's not do this in this bug

::: toolkit/webapps/WebappOSUtils.jsm
@@ +95,5 @@
> +
> +    try {
> +      if (exeFile.exists()) {
> +        var process = Components.classes["@mozilla.org/process/util;1"]  
> +                                        .createInstance(Components.interfaces.nsIProcess);

you can use Cc and Ci here
Comment 5 Marco Castelluccio [:marco] 2012-08-07 17:46:36 PDT
Created attachment 649899 [details] [diff] [review]
Patch
Comment 6 :Felipe Gomes (needinfo me!) 2012-08-08 17:20:21 PDT
Comment on attachment 649899 [details] [diff] [review]
Patch

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

great :)
Comment 7 :Felipe Gomes (needinfo me!) 2012-08-08 20:54:41 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcd9fb49866b
Comment 8 Ed Morley [:emorley] 2012-08-09 04:52:07 PDT
https://hg.mozilla.org/mozilla-central/rev/bcd9fb49866b
Comment 9 Marco Castelluccio [:marco] 2012-08-12 09:37:48 PDT
Comment on attachment 649899 [details] [diff] [review]
Patch

[Approval Request Comment]
User impact if declined: Users won't be able to automatically uninstall applications.
Testing completed (on m-c, etc.): On central for some days.
Risk to taking this patch (and alternatives if risky): No risk.
String or UUID changes made by this patch: None.

We need also the patch in bug 761806.
Comment 10 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-15 10:10:37 PDT
Comment on attachment 649899 [details] [diff] [review]
Patch

same thing as bug 761806, this is a wontfix for 16 and can ride the trains.

Note You need to log in before you can comment on or make changes to this bug.