Closed
Bug 797574
Opened 12 years ago
Closed 12 years ago
PermissionsInstaller.jsm broken for package apps
Categories
(Core Graveyard :: DOM: Apps, defect)
Core Graveyard
DOM: Apps
Tracking
(blocking-basecamp:+, firefox18 fixed)
Tracking | Status | |
---|---|---|
firefox18 | --- | fixed |
People
(Reporter: dchanm+bugzilla, Assigned: fabrice)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file)
(deleted),
patch
|
gwagner
:
review+
|
Details | Diff | Splinter Review |
Package app installation was recently changed by bug 789527 . This invalidated one of the assumptions that PermissionsInstaller relied on.
app.manifest is no longer defined for packaged apps [1]
The manifest now resides at app.updateManifest. The Webapps.jsm code accounts for this [2]. Other parts of the code that rely on ManifestHelper may break without a similar isPackage check.
Dump from PermissionsInstaller.jsm before ManifestHelper is called
I/Gecko ( 438): app is {"installOrigin":"http://mochi.test:8888","origin":"app://{79c967f9-30e1-4d20-99aa-b7b6aa1f6058}","manifestURL":"http://example.com/tests/webapi/apps/webapp.sjs","updateManifest":{"name":"Super Crazy Basic App","installs_allowed_from":["*"],"type":"certified","package_path":"http://example.com/tests/webapi/app.zip","permissions":{"geolocation":{"description":"geolocate"},"contacts":{"description":"contacts","access":"readwrite"}}},"etag":null,"receipts":[],"categories":[],"removable":true,"id":"{79c967f9-30e1-4d20-99aa-b7b6aa1f6058}","installTime":1349300320014,"lastUpdateCheck":1349300320015}
[1] - http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.js#229
[2] - http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#897
Reporter | ||
Updated•12 years ago
|
Summary: PermissionInstaller.jsm broken for package apps → PermissionsInstaller.jsm broken for package apps
Assignee | ||
Comment 1•12 years ago
|
||
So, the question is: do we want to have the permissions in the mini-manifest and install them from here, or do we wait for the package download to use the "normal" manifest?
Blocks: 780955
We should not be putting the permissions in the minimanifest. And absolutely not be making PermissionsInstaller.jsm use permissions enumerated in the mini manifest.
The list of permissions granted to an app needs to be signed by the store. This can only happen in the package.
This is exactly why we don't want to put permissions into the mini manifest, even for informative purposes, since there's a risk that someone would actually use it to make security decisions.
So we can't go through the permission-installation code until the package has been downloaded.
Updated•12 years ago
|
Assignee: nobody → fabrice
blocking-basecamp: ? → +
Assignee | ||
Comment 3•12 years ago
|
||
With this patch we update permissions in the following situations:
- Packaged apps: we have downloaded the package or applied a new update.
- Hosted apps: the installation is confirmed, or we updated.
For hosted apps with an app-cache, I'm not sure whether to defer the permissions install/update up to the moment the app-cache is downloaded.
Attachment #668588 -
Flags: review?(anygregor)
Comment 4•12 years ago
|
||
What patch is this based on? The argument list for InstallPermissions doesn't match with mxr.
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Gregor Wagner [:gwagner] from comment #4)
> What patch is this based on? The argument list for InstallPermissions
> doesn't match with mxr.
I build the aApp object with only the properties used (.manifest, .orgin and .manifestURL, and the 3rd parameter is optional. I will update the function documentation though since it doesn't match the implementation anymore.
Updated•12 years ago
|
Attachment #668588 -
Flags: review?(anygregor) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Comment 7•12 years ago
|
||
Updated•12 years ago
|
status-firefox18:
--- → fixed
Comment 8•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Updated•12 years ago
|
QA Contact: jsmith
Updated•12 years ago
|
QA Contact: jsmith
Updated•7 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•