[webkit-reviews] review granted: [Bug 236301] Disable non yet functional permission API : [Attachment 451248] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 236301: Disable non yet functional permission API
https://bugs.webkit.org/show_bug.cgi?id=236301

Attachment 451248: Patch

https://bugs.webkit.org/attachment.cgi?id=451248&action=review




--- Comment #4 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 451248
  --> https://bugs.webkit.org/attachment.cgi?id=451248
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=451248&action=review

r=me

>>> Source/WebCore/ChangeLog:3
>>> +	     Disable non yet functional permission API
>> 
>> non -> not ?
>> 
>> Also, don't you mean that you are un-exposing it to workers?
>> 
>> Also, while you unexposed self.PermissionStatus and self.Permissions, I
*think* that navigator.permissions is still accessible for workers, no?
> 
> AFAIK, there is no WorkerNavigator+Permissions.idl so it is not exposed in
workers.

That is a good point. I missed that.

I still think we should clarify the bug title / change log description to match
what the patch actually does.

Right now, it makes it sound like you're disabling the permission API entirely.

> Source/WebCore/Modules/permissions/PermissionStatus.idl:31
> +    Exposed=(Window)

Maybe we should add a FIXME to indicate this would be exposed to workers.

> Source/WebCore/Modules/permissions/Permissions.idl:31
> +    Exposed=(Window)

ditto.