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

tobie commented on this pull request.

Thanks a lot for making this. Overall this looks really solid! Couple of comments and questions below (this is a part of the spec I'm not super familiar with, so please bear with me if some of those are silly).

> +
+    When the \[[OwnPropertyKeys]] internal method of a [=legacy platform object=] |O| is called,
+    the following steps are taken:
+
+    1.  Let |keys| be a new empty [=list=] of ECMAScript String and Symbol values.
+    1.  If |O| [=support indexed properties|supports indexed properties=], then [=list|for each=] |index| of |O|’s
+        [=supported property indices=], in ascending numerical order, [=list|append=] [=!=] [=ToString=](|index|) to
+        |keys|.
+    1.  If |O| [=support named properties|supports named properties=], then [=list|for each=] |P| of |O|’s
+        [=supported property names=] that is visible according to the [=named property visibility algorithm=],
+        [=list|append=] |P| to |keys|.
+    1.  [=list|For each=] |P| of |O|’s own property keys that is a String, in ascending chronological order of
+        property creation, [=list|append=] |P| to |keys|.
+    1.  [=list|For each=] |P| of |O|’s own property keys that is a Symbol, in ascending chronological order of
+        property creation, [=list|append=] |P| to |keys|.
+    1.  Assert: |keys| has no duplicate items.

Why not used an ordered list instead (as you initially) suggested?

> @@ -12014,19 +12014,26 @@ However, for [=legacy platform objects=],
 properties on the object must be
 enumerated in the following order:
 
-1.  If the object [=support indexed properties|supports indexed properties=], then
-    the object’s [=supported property indices=] are
-    enumerated first, in numerical order.
-1.  If the object [=support named properties|supports named properties=] and doesn't implement an [=interface=] with the
-    [{{LegacyUnenumerableNamedProperties}}]

The effects of the [{{LegacyUnenumerableNamedProperties}}] extended attribute seem totally gone from the updated algorithm. Is that on purpose?

> -1.  Finally, any enumerable own properties or properties from the object’s prototype chain are then enumerated,
-    in no defined order.
-
-Note: Future versions of the ECMAScript specification may define a total order for property enumeration.
+<div algorithm="to invoke the internal [[OwnPropertyKeys]] method of legacy platform objects">
+
+    When the \[[OwnPropertyKeys]] internal method of a [=legacy platform object=] |O| is called,
+    the following steps are taken:
+
+    1.  Let |keys| be a new empty [=list=] of ECMAScript String and Symbol values.
+    1.  If |O| [=support indexed properties|supports indexed properties=], then [=list|for each=] |index| of |O|’s
+        [=supported property indices=], in ascending numerical order, [=list|append=] [=!=] [=ToString=](|index|) to
+        |keys|.
+    1.  If |O| [=support named properties|supports named properties=], then [=list|for each=] |P| of |O|’s
+        [=supported property names=] that is visible according to the [=named property visibility algorithm=],
+        [=list|append=] |P| to |keys|.

Are these also in ascending chronological order?

> @@ -12014,19 +12014,26 @@ However, for [=legacy platform objects=],
 properties on the object must be

Would be great to add a small blurb here that this is done by the [[OwnPropertyKeys]] method, e.g: 

```
This document does not define a complete property enumeration order
for [=platform objects=] implementing [=interfaces=]
(or for <a href="#es-exception-objects">platform objects representing exceptions</a>).
However, it does for [=legacy platform objects=] by defining the \[[OwnPropertyKeys]] method as follows:
```

-- 
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#pullrequestreview-54618522