Closed
Bug 1179718
Opened 9 years ago
Closed 9 years ago
Add a CheckAllPermissions extended attribute to WebIDL
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: kanru, Assigned: kanru)
References
Details
Attachments
(3 files)
The CheckPermissions extended attribute implemented in bug 952486 have a "or" relationship of the listed permissions; that is, any one of the listed permission is enough for the interface or interface member to be exposed. I'd like to add a new CheckAllPermissions attribute that enforces "and" relationship of the listed permissions. So the content has to have all the listed permissions for the interface or interface member to be exposed. We already have some hierarchical permissions implemented but couldn't use WebIDL to express them instead have to check the sub-permission in code.
Comment 1•9 years ago
|
||
Sounds reasonable to me. kanru, do you want to do this yourself?
Assignee | ||
Comment 2•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #1) > Sounds reasonable to me. kanru, do you want to do this yourself? Ah, I want to give it a try, it should be interesting.
Assignee | ||
Comment 3•9 years ago
|
||
Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz
Attachment #8629260 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r?bz
Attachment #8629261 -
Flags: review?(bzbarsky)
Updated•9 years ago
|
Attachment #8629260 -
Flags: review?(bzbarsky)
Comment 5•9 years ago
|
||
Comment on attachment 8629260 [details] MozReview Request: Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz https://reviewboard.mozilla.org/r/12545/#review11153 ::: dom/bindings/BindingUtils.h:3135 (Diff revision 1) > CheckPermissions(JSContext* aCx, JSObject* aObj, const char* const aPermissions[]); I think we should rename the existing CheckPermissions to CheckAnyPermissions to make it clearer what's going on. Followup patch is ok for this if you're prefer that. ::: dom/bindings/Codegen.py:11454 (Diff revision 1) > + if len(descriptor.allpermissions): Seems like this could be factored out into a function that takes the "anypermissions" or "allpermissions" string and does the right thing... ::: dom/bindings/Configuration.py:569 (Diff revision 1) > + # Adds a allpermission list to this descriptor and returns the index to use. This should _definitely_ be factored out some. There's some code duplication in the parser too that might make sense to factor out; I can't tell for sure from these diffs.
Comment 6•9 years ago
|
||
Comment on attachment 8629261 [details] MozReview Request: Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r=bz https://reviewboard.mozilla.org/r/12547/#review11159 Ship It!
Attachment #8629261 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8629260 [details] MozReview Request: Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz
Attachment #8629260 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8629261 [details] MozReview Request: Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r=bz Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r=bz
Attachment #8629261 -
Attachment description: MozReview Request: Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r?bz → MozReview Request: Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r=bz
Attachment #8629261 -
Flags: review+
Assignee | ||
Comment 9•9 years ago
|
||
Bug 1179718 - Rename CheckPermissions to CheckAnyPermissions. r?bz
Attachment #8633277 -
Flags: review?(bzbarsky)
Comment 10•9 years ago
|
||
Comment on attachment 8629260 [details] MozReview Request: Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz https://reviewboard.mozilla.org/r/12545/#review11867 ::: dom/bindings/Codegen.py:1903 (Diff revision 2) > + assert checkAllPermissions is None or isinstance(checkAllPermissions, int) Extra space after "None" here. Please remove it. ::: dom/bindings/Codegen.py:11451 (Diff revision 2) > - for (k, v) in sorted(descriptor.permissions.items()): > + permissions = getattr(descriptor, name, None) I don't think you need to pass the None there. The descriptor will always have the attr, no? ::: dom/bindings/Configuration.py:553 (Diff revision 2) > - def addPermissions(ifaceOrMember): > + def addPermissions(ifaceOrMember, checkAll=False): You only have two callers, one of which passes true and the other passes false. Just make checkAll a required argument. Also, it's probably better to make it the "attribute" string and then just set "permissions" based on that. Avoids the weird boolean argument thing where you can't tell what it means at the callsite. ::: dom/bindings/parser/WebIDL.py:1177 (Diff revision 2) > + if (self.getExtendedAttribute("CheckAllPermissions") and This should really be changed to have a list of primary-global-only attributes (just two of them for now?) and walk it, raising exceptions as needed. ::: dom/bindings/parser/WebIDL.py:3381 (Diff revision 2) > + if (self.getExtendedAttribute("CheckAllPermissions") and And here too, should just have a list of primary-global-only things.
Attachment #8629260 -
Flags: review?(bzbarsky)
Comment 11•9 years ago
|
||
Comment on attachment 8633277 [details] MozReview Request: Bug 1179718 - Rename CheckPermissions to CheckAnyPermissions. r=bz https://reviewboard.mozilla.org/r/13195/#review11881 Ship It!
Attachment #8633277 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8629260 [details] MozReview Request: Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz
Attachment #8629260 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8629261 [details] MozReview Request: Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r=bz Bug 1179718 - Convert BrowserElement.webidl to use CheckAllPermissions. r=bz
Assignee | ||
Updated•9 years ago
|
Attachment #8633277 -
Attachment description: MozReview Request: Bug 1179718 - Rename CheckPermissions to CheckAnyPermissions. r?bz → MozReview Request: Bug 1179718 - Rename CheckPermissions to CheckAnyPermissions. r=bz
Attachment #8633277 -
Flags: review+
Assignee | ||
Comment 14•9 years ago
|
||
Comment on attachment 8633277 [details] MozReview Request: Bug 1179718 - Rename CheckPermissions to CheckAnyPermissions. r=bz Bug 1179718 - Rename CheckPermissions to CheckAnyPermissions. r=bz
Comment 15•9 years ago
|
||
Comment on attachment 8629260 [details] MozReview Request: Bug 1179718 - Add a CheckAllPermissions extended attribute to WebIDL. r?bz https://reviewboard.mozilla.org/r/12545/#review12093 Ship It!
Attachment #8629260 -
Flags: review?(bzbarsky) → review+
Comment 16•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7c07736ee7a https://hg.mozilla.org/integration/mozilla-inbound/rev/009f8711fd6c https://hg.mozilla.org/integration/mozilla-inbound/rev/4dd71c32a6cb
Comment 17•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c7c07736ee7a https://hg.mozilla.org/mozilla-central/rev/009f8711fd6c https://hg.mozilla.org/mozilla-central/rev/4dd71c32a6cb
Assignee: nobody → kanru
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•