Re: [heycam/webidl] Make interface prototype object creation imperative (#494)

domenic approved this pull request.

Looks great! Only a few minor suggestions; feel free to merge without another round.

> +        1.  [=list/For each=] [=exposed=] [=member=] |member| of |interface|
+            that is declared with the [{{Unscopable}}] [=extended attribute=]:
+            1.  Let |id| be |member|'s [=identifier=].
+            1.  Perform [=!=] <a abstract-op>CreateDataProperty</a>(|unscopableObject|, |id|,
+                <emu-val>true</emu-val>).
+        1.  Let |desc| be the PropertyDescriptor{\[[Value]]: |unscopableObject|,
+            \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>,
+            \[[Configurable]]: <emu-val>true</emu-val>}.
+        1.  Perform [=!=] <a abstract-op>DefinePropertyOrThrow</a>(|interfaceProtoObj|, {{@@unscopables}}, |desc|).
+    1.  If |interface| is declared with the [{{Global}}] [=extended attribute=], or
+        |interface| is in the set of [=inherited interfaces=] of an interface
+        that is declared with the [{{Global}}] [=extended attribute=], then:
+        1.  Set the internal methods of |interfaceProtoObj|
+            which are specific to [=immutable prototype exotic objects=]
+            to the definitions specified in
+            [=ECMA-262 §9.4.7|ECMA-262 §9.4.7 Immutable prototype exotic objects=].

The section numbers here seem likely to get out of date; perhaps just "ECMA-262 Immutable prototype exotic objects"?

> +        then set |proto| to the [=named properties object=] in |realm| of |interface|.
+    1.  Otherwise, if |interface| is declared to inherit from another interface,
+        then set |proto| to the [=interface prototype object=] in |realm|
+        of that [=inherited interface=].
+    1.  Otherwise, if |interface| is the {{DOMException}} [=interface=],
+        then set |proto| to |realm|.\[[Intrinsics]].[[{{%ErrorPrototype%}}]].
+    1.  Otherwise, if |interface| is declared with the [{{LegacyArrayClass}}] [=extended attribute=],
+        then set |proto| to |realm|.\[[Intrinsics]].[[{{%ArrayPrototype%}}]].
+    1.  Otherwise, set |proto| to |realm|.\[[Intrinsics]].[[{{%ObjectPrototype%}}]].
+    1.  Assert: <a abstract-op>Type</a>(|proto|) is Object.
+    1.  Let |interfaceProtoObj| be [=!=] <a abstract-op>ObjectCreate</a>(|proto|).
+    1.  If |interface| has any [=member=] declared with the [{{Unscopable}}] [=extended attribute=],
+        then:
+
+        Issue: Should an {{@@unscopables}} property also be defined if |interface| is
+        declared with the [{{Global}}] [=extended attribute=]?

Can we file this in the issue tracker? I don't remember why it would be done.

> -    the following steps:
-
-    1.  If |A| is declared to inherit from another interface, then return the
-        [=interface prototype object=]
-        for the inherited interface.
-    1.  Otherwise, if |A| is declared with the [{{LegacyArrayClass}}]
-        extended attribute, then return {{%ArrayPrototype%}}.
-    1.  Otherwise, return {{%ObjectPrototype%}}.
+<div algorithm>
+    The [=named properties object=] for a given [=interface=] |interface| and [=Realm=] |realm|,
+    is <dfn lt="create a named properties object">created</dfn> as follows:
+
+    1.  Let |proto| be null.
+    1.  If |interface| is declared to inherit from another interface,
+        then set |proto| to the [=interface prototype object=] in |realm| for the [=inherited interface=].
+    1.  Otherwise, if |interface| is declared with the [{{LegacyArrayClass}}] [=extended attribute=],

May be worth a bug to disallow [LegacyArrayClass] on [Global]s so we can delete this.

> @@ -12407,6 +12468,35 @@ attribute is the concatenation of the class string of the namespace, ".",
 and the class string that the interface would otherwise have without this
 extended attribute.
 
+[=Platform objects=] have an internal \[[SetPrototypeOf]] method
+as defined in [[#platform-object-setprototypeof]].
+
+For each [=interface=] |interface| implemented by a [=platform object=] |obj|,
+the following steps must be run:
+
+1.  [=Define the unforgeable regular operations=] of |interface| on |obj|, given [=Realm=] |realm|.

_realm_ is not defined. Probably just need to add it into the "For each" statement, e.g. "Within a Realm _realm_, for each...". Similar problem below.

> @@ -12407,6 +12468,35 @@ attribute is the concatenation of the class string of the namespace, ".",
 and the class string that the interface would otherwise have without this
 extended attribute.
 
+[=Platform objects=] have an internal \[[SetPrototypeOf]] method
+as defined in [[#platform-object-setprototypeof]].
+
+For each [=interface=] |interface| implemented by a [=platform object=] |obj|,
+the following steps must be run:

Maybe "run as part of _obj_'s creation"?

> +For each [=interface=] |interface| implemented by a [=platform object=] |obj|,
+the following steps must be run:
+
+1.  [=Define the unforgeable regular operations=] of |interface| on |obj|, given [=Realm=] |realm|.
+1.  [=Define the unforgeable regular attributes=] of |interface| on |obj|, given [=Realm=] |realm|.
+
+If a [=platform object=] |obj| implements an interface
+which is declared with the [{{Global}}] [=extended attribute=], then
+for each [=interface=] |interface| implemented by a |obj|,
+the following steps must be run:
+
+1.  [=Define the regular operations=] of |interface| on |obj|, given [=Realm=] |realm|.
+1.  [=Define the regular attributes=] of |interface| on |obj|, given [=Realm=] |realm|.
+
+Additionaly, [=platform objects=] which implement an [=interface=]
+which has a [{{Global}}] [=extended attribute=],

I think this comma is unnecessary.

-- 
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/494#pullrequestreview-110579367

Received on Monday, 9 April 2018 19:22:08 UTC