The default bug view has changed. See this FAQ.

getNotInstalled should be under mgmt and return apps from all origins

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Apps
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: anant, Assigned: \)

Tracking

({dev-doc-needed})

unspecified
mozilla17
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
In bug 768276 the getNotInstalled function was added, but directly under navigator.mozApps. It should be under navigator.mozApps.mgmt, which signifies APIs that are "privileged".

Both getNotInstalled and getAll fall under the privileged category, while getInstalled can stay directly under mozApps since it only returns apps that were installed from the caller's origin.
Summary: getNotInstalled should be under mgmt → getNotInstalled should be under mgmt and return apps from all origins

Updated

5 years ago
Keywords: dev-doc-needed
Assignee: nobody → ndesaulniers
Code is functional (I think, but I'll be more confident after unit tests), and I've fixed regressions with the other tests.  Now just to write up a chrome mochitest (not a browser mochitest)!

test with:
TEST_PATH=dom/tests/mochitest/webapps/ make -C objdir/ mochitest-chrome
Depends on: 783391
Tests are green.  Rebasing, and running tests again to double check everything, then submitting patch for review.
Created attachment 653436 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins
Attachment #653436 - Flags: review?(felipc)
Comment on attachment 653436 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins

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

::: dom/tests/mochitest/webapps/test_getNotInstalled.xul
@@ +17,5 @@
> +
> +let _isLaunchable;
> +let steps = [
> +  cleanUp, installApp, monkeyPatchDOMApplicationRegistry, getNotInstalled,
> +  cleanUp, unmonkeyPatchDOMApplicationRegistry, tearDown

I believe you need to undo the monkey patching before calling cleanUp, otherwise the app you installed won't be uninstalled because the getAll used in uninstallAll will skip it
Created attachment 653509 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins
Attachment #653436 - Attachment is obsolete: true
Attachment #653436 - Flags: review?(felipc)
Attachment #653509 - Flags: review?(felipc)
(In reply to :Felipe Gomes from comment #4)
> Comment on attachment 653436 [details] [diff] [review]
> getNotInstalled should be under mgmt and return apps from all origins
> 
> Review of attachment 653436 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/tests/mochitest/webapps/test_getNotInstalled.xul
> @@ +17,5 @@
> > +
> > +let _isLaunchable;
> > +let steps = [
> > +  cleanUp, installApp, monkeyPatchDOMApplicationRegistry, getNotInstalled,
> > +  cleanUp, unmonkeyPatchDOMApplicationRegistry, tearDown
> 
> I believe you need to undo the monkey patching before calling cleanUp,
> otherwise the app you installed won't be uninstalled because the getAll used
> in uninstallAll will skip it

Good catch, I reordered the steps in the test.
Attachment #653509 - Flags: review?(felipc) → review+
(Reporter)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e952e6793ae
Comment on attachment 653509 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins

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

Please file a follow-up or backout this patch.

::: dom/interfaces/apps/nsIDOMApplicationRegistry.idl
@@ +54,5 @@
>    /**
> +   * the request will return the applications acquired from all origins but
> +   * which are not launchable (e.g. by not being natively installed), or null.
> +   */
> +  nsIDOMDOMRequest getNotInstalled();

You must change the UUID of the interface.

@@ -95,5 @@
>    /**
> -   * the request will return the applications acquired from this origin but which
> -   * are not launchable (e.g. by not being natively installed), or null.
> -   */
> -  nsIDOMDOMRequest getNotInstalled();

And this one also.
Attachment #653509 - Flags: review+ → review-
(Reporter)

Comment 9

5 years ago
Comment on attachment 653509 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins

Bug 784245 is the follow-up.
Attachment #653509 - Flags: review-
https://hg.mozilla.org/mozilla-central/rev/3e952e6793ae
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.