Closed
Bug 775199
Opened 12 years ago
Closed 12 years ago
Contacts API: remove whitelist approach for permissions.
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: gwagner, Assigned: gwagner)
References
Details
Attachments
(2 files)
(deleted),
patch
|
mounir
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
Comment on attachment 643481 [details] [diff] [review] patch The // XXX REMOVEME part should be removed once all related gaia changes landed as well.
Attachment #643481 -
Flags: review?(mounir)
Assignee | ||
Updated•12 years ago
|
blocking-basecamp: --- → ?
Comment 3•12 years ago
|
||
Comment on attachment 643481 [details] [diff] [review] patch Review of attachment 643481 [details] [diff] [review]: ----------------------------------------------------------------- So, with that patch alone, no app will have contacts permissions, right? We will need to have them added at install time/generated in the profile. Is that correct? r=me in that case and with the following comments applied: ::: dom/contacts/ContactManager.js @@ +457,5 @@ > let secMan = Cc["@mozilla.org/scriptsecuritymanager;1"].getService(Ci.nsIScriptSecurityManager); > > let perm = principal == secMan.getSystemPrincipal() > ? Ci.nsIPermissionManager.ALLOW_ACTION > + : Services.perms.testExactPermissionFromPrincipal(principal, "contacts"); Actually, you can change all that too: let perm = Services.perms.testExactPermissionFromPrincipal(aWindow.document.nodePrincipal, "contacts"); we are now always returning ALLOW_ACTION for the system principal. @@ +465,5 @@ > + debug("Contacts permission: " + this.hasPrivileges); > + > + // XXX REMOVEME > + if(!this.hasPrivileges) { > + dump("WARNING: contacts permssion missing! Setting to true!"); Remove that. As Vivien said, we should just push data changes when Gecko changes land.
Comment 4•12 years ago
|
||
Comment on attachment 643481 [details] [diff] [review] patch Actually meant r=me... see the previous comment.
Attachment #643481 -
Flags: review?(mounir) → review+
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Mounir Lamouri (:mounir) from comment #3) > Comment on attachment 643481 [details] [diff] [review] > patch > > Review of attachment 643481 [details] [diff] [review]: > ----------------------------------------------------------------- > > So, with that patch alone, no app will have contacts permissions, right? We > will need to have them added at install time/generated in the profile. Is > that correct? So with this patch we still grant permission in the XXX Remove part but without the removeme part we rely on the pre-population.
Comment 6•12 years ago
|
||
Just make sure to land the prepopulation part first and land that patch after *without* the "XXX REMOVEME" part.
Updated•12 years ago
|
blocking-basecamp: ? → +
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/52a37789b6c4
Assignee | ||
Comment 9•12 years ago
|
||
And the review comment https://hg.mozilla.org/integration/mozilla-inbound/rev/a19d59d6e3a6
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/52a37789b6c4 https://hg.mozilla.org/mozilla-central/rev/a19d59d6e3a6
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•