Re: [heycam/webidl] Define [[OwnPropertyKeys]] of legacy platform objects (#402)

I've completed a more comprehensive audit of what browsers currently do. And the results are, well, inconsistently inconsistent. The test is live at https://timothygu.me/webidl-keys-test/, and the files available in https://github.com/TimothyGu/webidl-keys-test.

I specifically looked at three interfaces: [NamedNodeMap](https://dom.spec.whatwg.org/#interface-namednodemap) (used for `element.attributes`), [NodeList](https://dom.spec.whatwg.org/#nodelist) (through `node.childNodes`), and [Storage](https://html.spec.whatwg.org/multipage/webstorage.html#storage-2) (through `window.sessionStorage`). For browsers, I checked Chrome 60 (stable) and 61 (beta), Firefox 55 beta, and Edge 15. I couldn't get used to Safari's developer console, but at a glance its behavior is fairly similar to Chrome's.

----

>From results from `NamedNodeMap` and `NodeList`:

Chrome and Edge both exhibit the original issue I opened #400 for, namely they do not include named properties on an interface annotated with `[LegacyUnenumerableNamedProperties]` in `Reflect.ownKeys()`, while Firefox does. This PR follows the Firefox behavior, as `[LegacyUnenumerableNamedProperties]` should only set the `enumerable` bit rather than hide it from getting all property names.

While Chrome and Firefox (and this proposal) consistently put own property symbols at the end for legacy platform objects in the same way as ES operates, Edge puts symbols in the front of all own properties.

Not directly related to this proposal, Chrome 60 and Edge follow the current [[Set]] semantics in that for interfaces not supporting indexed properties, it still sets any indexed properties as own properties. However, Chrome 61 and Firefox will throw when trying to do so.

Also not directly related to this proposal, Edge has a bug where its `Reflect.ownKeys()` returns duplicated property names when an object has both an own property and a supported indexed property [link](https://github.com/TimothyGu/webidl-keys-test/blob/480437a2a1152dbc5b705376cdcc5458a9632444/test-named-node-map.js#L82). In that situation, when getting the duplicated key it gets the own property instead. Chrome 60 does it correctly, but the enumeration order is a bit off. Chrome 61 and Firefox did not get to this test because it threw an error when trying to set the own property earlier.

----

As for `Storage`, [all hell broke loose](https://github.com/TimothyGu/webidl-keys-test/blob/480437a2a1152dbc5b705376cdcc5458a9632444/test-storage.js). No pairs of browsers do the same, and no browsers implement the spec for [[Set]]. It does reveal an [issue](https://github.com/TimothyGu/webidl-keys-test/blob/480437a2a1152dbc5b705376cdcc5458a9632444/test-storage.js#L56-L61) in [[Set]] that we can fix fairly easily, but other than that there is no agreement. It also reveals browsers have subtly different [[Set]] semantics re. [forbidden property names (prototype method shadowing)](https://github.com/TimothyGu/webidl-keys-test/blob/480437a2a1152dbc5b705376cdcc5458a9632444/test-storage.js#L14-L27) and [symbol properties](https://github.com/TimothyGu/webidl-keys-test/blob/480437a2a1152dbc5b705376cdcc5458a9632444/test-storage.js#L29-L38). As such, it's impossible to properly derive a pattern for enumeration from the `Storage` interface we can reference in [[OwnPropertyKeys]].

----

>From the results, I see an even stronger need for standardization around [[OwnPropertyKeys]]. From the fact that both Chrome and Edge hide named properties from `Reflect.ownKeys()` for `[LegacyUnenumerableNamedProperties]` while Firefox and this proposal do not (i.e. the original #400), I'm open for some discussion regarding that. Other than this, I don't see anything that should be changed in this PR.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/402#issuecomment-320840117