[app manager] once an app has been installed, it doesn't show up in the "installed app" tab

VERIFIED FIXED in Firefox 26

Status

defect
P1
normal
VERIFIED FIXED
6 years ago
Last year

People

(Reporter: paul, Assigned: paul)

Tracking

Trunk
Firefox 27
x86
All
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox26 verified, firefox27 verified, b2g-v1.2 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

No description provided.
Priority: -- → P1
Assignee: nobody → paul
Depends on: 919981
Whiteboard: [needed-in-aurora-26]
Posted patch Patch v1 (obsolete) — Splinter Review
Attachment #809778 - Flags: review?(poirot.alex)
Comment on attachment 809778 [details] [diff] [review]
Patch v1

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

Looks good, but we can easily write a test for the new actor function.
See getAll test:
  http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/apps/tests/unit/test_webappsActor.js#36

You can run this test on firefox desktop by enabling the test:
  http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/apps/tests/unit/xpcshell.ini
and commenting two icon related tests.

::: browser/devtools/app-manager/webapps-store.js
@@ +10,5 @@
>  const dbgClient = Cu.import("resource://gre/modules/devtools/dbg-client.jsm");
>  dbgClient.UnsolicitedNotifications.appOpen = "appOpen";
>  dbgClient.UnsolicitedNotifications.appClose = "appClose"
> +dbgClient.UnsolicitedNotifications.appInstall = "appInstall";
> +dbgClient.UnsolicitedNotifications.appUninstall = "appUninstall";

In bug 919981's patch, I'm registering these notifications, and also appOpen/Close in dbg-client.jsm, so I think we no longer need them here.

@@ +224,5 @@
> +    };
> +
> +    client.request(request, (res) => {
> +      if (res.error) {
> +        return deferred.reject(res.error);

There is no `deferred` here. we should throw or console.error this message.

::: toolkit/devtools/server/actors/webapps.js
@@ +551,5 @@
> +    }
> +
> +    let deferred = promise.defer();
> +    let reg = DOMApplicationRegistry;
> +    let app = reg.getAppByManifestURL(manifestURL);

getAppByManifestURL isn't consistent with getAll. We miss the app manifest. app.manifest should be the app manifest, as a JSON object, but here, it's undefined.

We can fetch it like this:
  let deferred = promise.defer();
  reg.getManifestFor(manifestURL, function (manifest) {
    app.manifest = manifest;
    deferred.resolve({app: app});
  });
  return deferred.promise;
Attachment #809778 - Flags: review?(poirot.alex)
Posted patch Patch v2 (obsolete) — Splinter Review
Attachment #809778 - Attachment is obsolete: true
Attachment #810511 - Flags: review?(poirot.alex)
Comment on attachment 810511 [details] [diff] [review]
Patch v2

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

Works great!
Attachment #810511 - Flags: review?(poirot.alex) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/a2601d3f3214
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [needed-in-aurora-26] → [needed-in-aurora-26][fixed-in-fx-team]
Whiteboard: [needed-in-aurora-26][fixed-in-fx-team] → [needed-in-aurora-26]
Posted patch Patch V2.1 (obsolete) — Splinter Review
Whiteboard: [needed-in-aurora-26] → [blocked by bug 919981]
Posted patch Patch V3Splinter Review
Attachment #810511 - Attachment is obsolete: true
Attachment #811109 - Attachment is obsolete: true
Attachment #812710 - Flags: review?(poirot.alex)
Attachment #812710 - Flags: review?(poirot.alex) → review+
Keywords: checkin-needed
Whiteboard: [blocked by bug 919981] → [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/daf000728e1e
Keywords: checkin-needed
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/daf000728e1e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 27
Comment on attachment 812710 [details] [diff] [review]
Patch V3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new feature (app manager)
User impact if declined: list of installed apps not updated
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #812710 - Flags: approval-mozilla-aurora?
Whiteboard: [waiting-for-aurora-approval]
Attachment #812710 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0

Verified as fixed on Fx 26 beta 3 (20131107161719) and latest Aurora (20131108004000) using Firefox OS 1.2 or 1.3 Simulator.
Status: RESOLVED → VERIFIED
Keywords: verifyme
See also: Bug 986979
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.