Re: [heycam/webidl] Add an algorithm to create a platform object. (#635)

bzbarsky requested changes on this pull request.



> @@ -869,6 +871,19 @@ its inheritance hierarchy has a cycle.  That is, an interface
 |A| cannot inherit from itself, nor can it inherit from another
 interface |B| that inherits from |A|, and so on.
 
+<div algorithm>

I assume this can't just say "_I_ and _I_'s **inherited interfaces**" because we want a list, not a set?

> @@ -10699,14 +10714,18 @@ the <code>typeof</code> operator will return "function" when applied to an inter
         1.  Let &lt;|constructor|, |values|&gt; be the result
             of passing |S| and |args|.
             to the [=overload resolution algorithm=].
-        1.  Let |R| be the result of performing
-            the actions listed in the description of |constructor|
-            with |values| as the argument values.
+        1.  Let |targetRealm| be [$GetFunctionRealm$]({{NewTarget}}).
+        1.  Let |object| be the result of [=internally create a new object implementing the

This looks wrong to me.

We want to create the object in the current Realm of the constructor, period.  That's how ES builtins work, last I checked, and we should not be inventing a new behavior.  All we should be using NewTarget for is the prototype object, which may end up coming from a different Realm.

In other words, we should be passing our current Realm, and then inside the "internally create" algorithm, if newTarget is not undefined, we can get the prototype from it, if an object use that, else fall back on the default proto in GetFunctionRealm(newTarget).  And fall back to the passed-in Realm if there is no newTarget.

>              to an ECMAScript [=interface type=] value |I|.
         1.  Assert: |O| is an object that implements |I|.
-        1.  Assert: |O|.\[[Realm]] is equal to |realm|.
+        1.  Assert: |O|.\[[Realm]] is equal to |targetRealm|.

Again, this is NOT the behavior we want.

> @@ -12346,12 +12365,62 @@ It is the responsibility of specifications using Web IDL to state
 which global environment (or, by proxy, which global object) each platform
 object is associated with.
 
+<div algorithm>
+  To <dfn export lt=new>create a new object implementing the interface</dfn> |interface|, with a
+  Realm |realm|, perform the following steps:
+
+    1.  Return the result of [=internally create a new object implementing the
+        interface|internally creating a new object implementing=] |interface|, with |realm| and
+        <emu-val>undefined</emu-val>.
+</div>
+
+<div algorithm>
+  To <dfn>internally create a new object implementing the interface</dfn> |interface|, with a
+  Realm |realm| and a JavaScript value |newTarget|, perform the following steps:
+
+    1.  Assert: |interface| is [=exposed=] in |realm|.

Just as a note: this assert does NOT hold with the current definition of constructor behavior.  Last I checked, you could have a constructor from a Realm where the interface is exposed and a NewTarget from a Realm where the interface is _not_ exposed (at least with [SecureContext], where things can be same-origin yet different secure context state, last I checked) and the NewTarget Realm is what's getting passed in.  Yet another reason to not do that.  ;)

> +  To <dfn export lt=new>create a new object implementing the interface</dfn> |interface|, with a
+  Realm |realm|, perform the following steps:
+
+    1.  Return the result of [=internally create a new object implementing the
+        interface|internally creating a new object implementing=] |interface|, with |realm| and
+        <emu-val>undefined</emu-val>.
+</div>
+
+<div algorithm>
+  To <dfn>internally create a new object implementing the interface</dfn> |interface|, with a
+  Realm |realm| and a JavaScript value |newTarget|, perform the following steps:
+
+    1.  Assert: |interface| is [=exposed=] in |realm|.
+    1.  Let prototype be the [=interface prototype object=] for |interface| in
+        |realm|.
+    1.  If |newTarget| is not <emu-val>undefined</emu-val>, then:

See above for my suggestions on how this should be modified.

-- 
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/635#pullrequestreview-203837976