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.

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

Add Symbol.unscopables lexical scope test of compiled event handlers #26091

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Oct 13, 2020

Step 9 of get the current value of the event handler initialises the ObjectEnvironment records without setting the withEnvironment flag to true, which is required to make the ObjectEnvironment respect Symbol.unscopables.

As such, inline event handlers are required to include unscopable properties.

See also:

@domenic
Copy link
Member

domenic commented Oct 13, 2020

All browsers fail this test, so it should probably be flipped, and the spec changed accordingly.

@annevk
Copy link
Member

annevk commented Oct 20, 2020

@ExE-Boss if there's a difference between setting withEnvironment on the innermost and most scopes it'd be good to have test coverage for that as well.

annevk added a commit to whatwg/html that referenced this pull request Jan 18, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@ExE-Boss ExE-Boss force-pushed the html/webappapis/scripting/events/compile-event-handler-symbol-unscopables branch 2 times, most recently from 4b2a5d2 to 95c8f9d Compare March 14, 2021 15:35
@ExE-Boss
Copy link
Contributor Author

@annevk I’ve now also added test cases for the element and formOwner scopes.

@ExE-Boss ExE-Boss force-pushed the html/webappapis/scripting/events/compile-event-handler-symbol-unscopables branch from 95c8f9d to 3a529ac Compare March 14, 2021 16:43
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

This looks great. Just some minor tidying nits.

And I guess I should file a bug on Safari for not having the same unscopables. I wonder if there's a test for that somewhere... (Turns out this is tested by IDL harness tests and Safari has some failures for Fullscreen.)

@annevk annevk merged commit 81dc74e into web-platform-tests:master Mar 16, 2021
@annevk
Copy link
Member

annevk commented Mar 16, 2021

(I decided to land this as everyone agrees this is the behavior these should have and we have a specification change pending that can land as soon as ECMAScript has had a slight refactoring.)

@ExE-Boss ExE-Boss deleted the html/webappapis/scripting/events/compile-event-handler-symbol-unscopables branch March 16, 2021 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants