Do not expose in WebIDL the parts of the permission API that are not yet supported

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

Some permission API (on change event, worker support) is not yet implemented.
Let's not expose the WebIDL part of it for now.

[youenn fablet]

Created attachment 451248 [details]
Patch

[Chris Dumez]

Comment on attachment 451248 [details]
Patch

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

> 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?

[youenn fablet]

(In reply to Chris Dumez from comment #2)
> Comment on attachment 451248 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=451248&action=review
> 
> > 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.

[Chris Dumez]

Comment on attachment 451248 [details]
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.

[youenn fablet]

I filed https://bugs.webkit.org/show_bug.cgi?id=236443 to keep track of permission API in workers.

[youenn fablet]

Created attachment 451534 [details]
Patch for landing

[EWS]

Committed r289536 (247067@main): <https://commits.webkit.org/247067@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451534 [details].

[Radar WebKit Bug Importer]

<rdar://problem/88756040>