Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add [Exposed=*] to expose on all globals #526

Merged
merged 3 commits into from Nov 2, 2021
Merged

Add [Exposed=*] to expose on all globals #526

merged 3 commits into from Nov 2, 2021

Conversation

littledan
Copy link
Collaborator

@littledan littledan commented Feb 22, 2018 •
edited by pr-preview bot

This patch adds support in WebIDL for declaring an interface available
in all contexts.

Closes #468

This PR follows the syntax and outline suggested by @tobie . Thanks for your clear instructions here.

Preview | Diff

tobie
tobie reviewed Feb 22, 2018
View reviewed changes
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Feb 23, 2018

I think before landing this we should try to address the question at #468 (comment).

tobie
tobie reviewed Feb 23, 2018
View reviewed changes
index.bs Outdated Show resolved Hide resolved
tobie
tobie reviewed Feb 23, 2018
View reviewed changes
index.bs Outdated Show resolved Hide resolved
@littledan littledan force-pushed the exposed-star branch 2 times, most recently from 35b291d to 2e23cfb Compare February 23, 2018 14:42
@littledan
Copy link
Collaborator Author

@annevk Thanks for making this link. I think we should work out an answer in conjunction with TC39 to this question, given that ECMAScript includes things that are pretty equivalent in power/appropriateness to put everywhere to TextEncoder and URL. I'm fine with letting this PR be blocked on this question.

@tobie tobie added the do not merge yet Pull request must not be merged per rationale in comment label Feb 26, 2018
@littledan littledan mentioned this pull request Oct 20, 2018
Closed
@littledan
Copy link
Collaborator Author

We should include the jsidl label on this issue, since all ES APIs will need to be exposed to all globals

@tobie tobie added the jsidl label Oct 20, 2018
@leobalter
Copy link

@littledan I believe we should follow up on this PR for the proper ShadowRealms integration, right?

@leobalter leobalter mentioned this pull request Sep 22, 2021
Open
22 tasks
@littledan
Copy link
Collaborator Author

We could add this for ShadowRealms, or we could just add [Exposed=ShadowRealm]. The choice depends on whether we want to go around adding lots of things to Worklets which are not currently exposed, and which we might decide to expose on ShadowRealm. In offline conversations with different HTML editors, I heard mixed opinions on this question, cc @domenic @annevk . Given that, I thought that it might be better to start with the more conservative approach of just adding things specifically to ShadowRealm.

@leobalter

This comment has been minimized.

Sign in to view
@domenic
Copy link
Member

domenic commented Sep 22, 2021

I think it'd be better to add [Exposed=*]? Saying that in worklets you have to access these APIs indirectly via shadow realms seems strange.

@annevk
Copy link
Member

annevk commented Sep 23, 2021

In an offline discussion tc39/ecma262#1120 came up, but since we did not change JavaScript to support minimal realms (i.e., a subset of what is defined in JavaScript), we should not do so here by excluding worklets. (As that would make worklets the new minimal realms due to organizational boundaries.)

As I understand it the primary motivation for having minimal realms in worklets was to avoid steering web developers down the wrong path, but in practice this hasn't been much of a problem.

This is a long way of saying that I agree with @domenic, but I thought it might be useful to add some context.

cc @padenot @codehag @Ms2ger

@littledan
Copy link
Collaborator Author

OK, thanks for the clarifications everyone, sounds like I was the confused/misunderstanding one. Does that mean we should move ahead with [Exposed=*]?

@littledan @Ms2ger
Add [Exposed=*] to expose on all globals
c330ccd 
This patch adds support in WebIDL for declaring an interface available
in all contexts.

Closes whatwg#468
@Ms2ger
Copy link
Member

Ms2ger commented Oct 1, 2021

Rebased and rephrased a bit - what's next here?

ExE-Boss
ExE-Boss reviewed Oct 1, 2021
View reviewed changes
index.bs Outdated Show resolved Hide resolved
@Ms2ger @ExE-Boss
Update index.bs
bde135a 
Co-authored-by: ExE Boss <3889017+ExE-Boss@users.noreply.github.com>
@Ms2ger Ms2ger requested a review from annevk October 7, 2021 15:04
@annevk annevk requested review from EdgarChen and removed request for annevk October 8, 2021 10:24
EdgarChen
EdgarChen approved these changes Oct 8, 2021
View reviewed changes
Copy link
Member

@EdgarChen EdgarChen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@Ms2ger Ms2ger requested a review from TimothyGu October 12, 2021 08:40
@Ms2ger Ms2ger mentioned this pull request Oct 12, 2021
Closed
14 tasks
This was referenced Oct 15, 2021
Merged
Merged
@saschanaz saschanaz mentioned this pull request Oct 29, 2021
Merged
4 tasks
@EdgarChen
Copy link
Member

@Ms2ger it would be helpful if you could make a list of interfaces which [Exposed=*] is proposed somewhere publicly. Thanks!

TimothyGu
TimothyGu approved these changes Nov 1, 2021
View reviewed changes
Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@saschanaz saschanaz mentioned this pull request Nov 2, 2021
Open
@Ms2ger
Copy link
Member

Ms2ger commented Nov 2, 2021

@Ms2ger it would be helpful if you could make a list of interfaces which [Exposed=*] is proposed somewhere publicly. Thanks!

Besides the ones already listed in tc39/proposal-shadowrealm#331, we might propose some parts of Crypto and Performance - please do let me know if you'd like me to present the list in some other way

@EdgarChen EdgarChen removed the do not merge yet Pull request must not be merged per rationale in comment label Nov 2, 2021
@EdgarChen EdgarChen merged commit 59dae6f into whatwg:main Nov 2, 2021
This was referenced Nov 2, 2021
Closed
Merged
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request Nov 3, 2021
@caitp
[WebIDL] Support [Exposed=*] extended attribute
8525381 
https://bugs.webkit.org/show_bug.cgi?id=231082

Reviewed by Chris Dumez.

Adds a shorthand to expose interfaces/attributes on Window, Workers*,
and the forthcoming ShadowRealm global object.

See whatwg/webidl#468 and
whatwg/webidl#526 for details.

* bindings/scripts/CodeGenerator.pm:
(shouldPropertyBeExposed):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateRuntimeEnableConditionalStringForExposed):
* bindings/scripts/IDLParser.pm:
(parseExtendedAttributeRest2):
* bindings/scripts/preprocess-idls.pl:
* bindings/scripts/test/AudioWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/DOMWindowConstructors.idl:
* bindings/scripts/test/DedicatedWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/ExposedStar.idl: Added.
* bindings/scripts/test/JS/JSDOMWindow.cpp:
(WebCore::jsDOMWindow_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSDedicatedWorkerGlobalScope.cpp:
(WebCore::jsDedicatedWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSExposedStar.cpp: Added.
(WebCore::JSExposedStarDOMConstructor::prototypeForStructure):
(WebCore::JSExposedStarDOMConstructor::initializeProperties):
(WebCore::JSExposedStarPrototype::finishCreation):
(WebCore::JSExposedStar::JSExposedStar):
(WebCore::JSExposedStar::finishCreation):
(WebCore::JSExposedStar::createPrototype):
(WebCore::JSExposedStar::prototype):
(WebCore::JSExposedStar::getConstructor):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
(WebCore::jsExposedStarPrototypeFunction_operationForAllContextsBody):
(WebCore::JSC_DEFINE_HOST_FUNCTION):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWindowContextsBody):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWorkerContextsBody):
(WebCore::JSExposedStar::subspaceForImpl):
(WebCore::JSExposedStar::analyzeHeap):
(WebCore::toJSNewlyCreated):
(WebCore::toJS):
(WebCore::JSExposedStar::toWrapped):
* bindings/scripts/test/JS/JSExposedStar.h: Added.
(WebCore::JSExposedStar::create):
(WebCore::JSExposedStar::createStructure):
(WebCore::JSExposedStar::subspaceFor):
(WebCore::JSExposedStar::wrapped const):
(WebCore::toJS):
(WebCore::toJSNewlyCreated):
* bindings/scripts/test/JS/JSPaintWorkletGlobalScope.cpp:
(WebCore::jsPaintWorkletGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSServiceWorkerGlobalScope.cpp:
(WebCore::jsServiceWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/PaintWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/ServiceWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/SupplementalDependencies.dep:



Canonical link: https://commits.webkit.org/243822@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285196 268f45cc-cd09-0410-ab3c-d52691b4dbfc
philn pushed a commit to philn/old-webkit that referenced this pull request Nov 3, 2021
[WebIDL] Support [Exposed=*] extended attribute
a92d3fd 
https://bugs.webkit.org/show_bug.cgi?id=231082

Reviewed by Chris Dumez.

Adds a shorthand to expose interfaces/attributes on Window, Workers*,
and the forthcoming ShadowRealm global object.

See whatwg/webidl#468 and
whatwg/webidl#526 for details.

* bindings/scripts/CodeGenerator.pm:
(shouldPropertyBeExposed):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateRuntimeEnableConditionalStringForExposed):
* bindings/scripts/IDLParser.pm:
(parseExtendedAttributeRest2):
* bindings/scripts/preprocess-idls.pl:
* bindings/scripts/test/AudioWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/DOMWindowConstructors.idl:
* bindings/scripts/test/DedicatedWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/ExposedStar.idl: Added.
* bindings/scripts/test/JS/JSDOMWindow.cpp:
(WebCore::jsDOMWindow_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSDedicatedWorkerGlobalScope.cpp:
(WebCore::jsDedicatedWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSExposedStar.cpp: Added.
(WebCore::JSExposedStarDOMConstructor::prototypeForStructure):
(WebCore::JSExposedStarDOMConstructor::initializeProperties):
(WebCore::JSExposedStarPrototype::finishCreation):
(WebCore::JSExposedStar::JSExposedStar):
(WebCore::JSExposedStar::finishCreation):
(WebCore::JSExposedStar::createPrototype):
(WebCore::JSExposedStar::prototype):
(WebCore::JSExposedStar::getConstructor):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
(WebCore::jsExposedStarPrototypeFunction_operationForAllContextsBody):
(WebCore::JSC_DEFINE_HOST_FUNCTION):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWindowContextsBody):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWorkerContextsBody):
(WebCore::JSExposedStar::subspaceForImpl):
(WebCore::JSExposedStar::analyzeHeap):
(WebCore::toJSNewlyCreated):
(WebCore::toJS):
(WebCore::JSExposedStar::toWrapped):
* bindings/scripts/test/JS/JSExposedStar.h: Added.
(WebCore::JSExposedStar::create):
(WebCore::JSExposedStar::createStructure):
(WebCore::JSExposedStar::subspaceFor):
(WebCore::JSExposedStar::wrapped const):
(WebCore::toJS):
(WebCore::toJSNewlyCreated):
* bindings/scripts/test/JS/JSPaintWorkletGlobalScope.cpp:
(WebCore::jsPaintWorkletGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSServiceWorkerGlobalScope.cpp:
(WebCore::jsServiceWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/PaintWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/ServiceWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/SupplementalDependencies.dep:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@285196 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@dontcallmedom dontcallmedom mentioned this pull request Jan 20, 2022
Merged
@legendecas legendecas mentioned this pull request Mar 30, 2022
Open
@littledan
Copy link
Collaborator Author

This change looks great, but I don't think it handles the ShadowRealm case yet, since that is not an interface.

@annevk
Copy link
Member

annevk commented Jun 8, 2022

@littledan that's a good point. I suppose we should put that requirement in HTML along with the overall ShadowRealm integration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobie tobie tobie left review comments

@ExE-Boss ExE-Boss ExE-Boss left review comments

@TimothyGu TimothyGu TimothyGu approved these changes

@EdgarChen EdgarChen EdgarChen approved these changes

Assignees
No one assigned
Labels
jsidl
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Exposing an interface "on all globals"
9 participants
@littledan @annevk @leobalter @domenic @Ms2ger @EdgarChen @tobie @TimothyGu @ExE-Boss