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

Named properties object: enumerate supported properties in [[OwnPropertyKeys]] #964

Conversation

shvaikalesh
Copy link
Contributor

Currently, Gecko implements proposed [[OwnPropertyKeys]] semantics, but Blink and WebKit do not.

This PR feels like an expected behavior and aligns named properties object with legacy platform objects on enumerating supported property names.

Since only Window has named properties object, and it also has a LegacyUnenumerableNamedProperties extended attribute, this change has no performance impact on for (var k in window) {}.

If this is feasible to implement in Blink, I will contribute extensive WPT coverage and author a patch for WebKit. Once #963 and [[OwnPropertyKeys]] discrepancies are resolved, WindowProperties will be 100% compatible across all implementations!

@annevk
Copy link
Member

annevk commented Mar 11, 2021

How would for (var k in window) {} end up invoking this, exactly? Wouldn't that end up invoking https://html.spec.whatwg.org/#windowproxy-ownpropertykeys?

@shvaikalesh
Copy link
Contributor Author

@annevk for/in indeed calls WindowProxy's [[OwnPropertyKeys]] first, but then goes up the prototype chain for same-origin windows.

WebKit, and I believe other runtimes as well, accept additional parameter when implementing [[OwnPropertyKeys]] so non-enumerable property names (for Object.keys and for/in) can be rejected without calling [[GetOwnProperty]].

@annevk
Copy link
Member

annevk commented Mar 11, 2021

Thank you! (I sometimes wonder if we should import more of this Window-specific logic into HTML, but that's also quite an ordeal.)

@shvaikalesh
Copy link
Contributor Author

shvaikalesh commented May 20, 2021

I've decided not to pursue this change: there is no clear benefit / use case to justify adding the method into 2 implementations, while other methods of named properties object remain unchanged.

Instead, to ensure interop, web-platform-tests/wpt#27970 adds a test that named properties objects has ordinary [[OwnPropertyKeys]] method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants