Add an API to obtain all permissions for a specific site from SitePermissions.jsm

RESOLVED FIXED in Firefox 49

Status

()

Firefox
General
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: johannh, Assigned: johannh)

Tracking

Trunk
Firefox 49
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [fxprivacy])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Quoting Paolo from Bug 1206245:

> If I remember correctly the discussions that led to this and other similar
> API bugs to be filed, we'd like to see a more structured approach to
> SitePermissions.jsm where the data model is more similar to the front-end
> model needed by the control center. One step in that direction could be to
> create a stateFromUri(uri) function that returns an object like this:
> 
> {
>   hasAnyGrantedPermission: true,
>   customizedPermissions: [
>     {
>       id: "...",
>       label: "...",
>       stateLabel: "...",
>     },
>     {
>       id: "...",
>       label: "...",
>       stateLabel: "...",
>       blocked: true,
>     },
>   ],
> }
> 
> And customizedPermissions would be an ordered list. This way, we could have
> simple non-ui regression tests to check that the model works, and we can
> keep the front-end code simpler, just dealing with rendering the controls
> and handling commands.
(Assignee)

Comment 1

2 years ago
Created attachment 8759132 [details]
MozReview Request: Bug 1277289 - Add a function to obtain all permissions by uri in SitePermissions.jsm. r=paolo

Review commit: https://reviewboard.mozilla.org/r/57196/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57196/
(Assignee)

Comment 2

2 years ago
Comment on attachment 8759132 [details]
MozReview Request: Bug 1277289 - Add a function to obtain all permissions by uri in SitePermissions.jsm. r=paolo

Looking at this patch we're actually increasing the code size but I think it still makes sense to put the moving parts into that function so that it can be verified to work independently of the UI.

Paolo, what do you think? I hope that this does what you were suggesting :)
Attachment #8759132 - Flags: feedback?(paolo.mozmail)

Comment 3

2 years ago
Comment on attachment 8759132 [details]
MozReview Request: Bug 1277289 - Add a function to obtain all permissions by uri in SitePermissions.jsm. r=paolo

Looks just like what we need here. Thanks!

>  // check that it returns an empty array on an invalid uri
>  let wrongURI = Services.io.newURI("file://example.js", null, null)

nit: I was confused for a bit by the reference to "invalid uri" because what is indicated here as "file://example.js" should be properly expressed as "file:///example.js". You should fix the URI and clarify the comment to indicate that we're testing an URI that doesn't support viewing permissions in the user interface.
Attachment #8759132 - Flags: feedback?(paolo.mozmail) → review+

Comment 4

2 years ago
Also, if I remember correctly it's fine to test English strings, I've seen similar tests around. I don't think we need to annotate the test in any way.
(Assignee)

Comment 5

2 years ago
Comment on attachment 8759132 [details]
MozReview Request: Bug 1277289 - Add a function to obtain all permissions by uri in SitePermissions.jsm. r=paolo

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57196/diff/1-2/
Attachment #8759132 - Attachment description: MozReview Request: Bug 1277289 - Add a function to obtain all permissions by uri in SitePermissions.jsm → MozReview Request: Bug 1277289 - Add a function to obtain all permissions by uri in SitePermissions.jsm. r=paolo
Attachment #8759132 - Flags: review+ → review?(paolo.mozmail)
(Assignee)

Updated

2 years ago
Attachment #8759132 - Flags: review?(paolo.mozmail) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Updated

2 years ago
Whiteboard: [fxprivacy] → [fxprivacy][triage]

Updated

2 years ago
Iteration: --- → 49.3 - Jun 6
Priority: -- → P1
Whiteboard: [fxprivacy][triage] → [fxprivacy]

Updated

2 years ago
Flags: qe-verify?

Comment 7

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/86dca8a132b5
Add a function to obtain all permissions by uri in SitePermissions.jsm. r=paolo
Keywords: checkin-needed
(Assignee)

Updated

2 years ago
Flags: qe-verify? → qe-verify-

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/86dca8a132b5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
You need to log in before you can comment on or make changes to this bug.