- From: Tobie Langel <notifications@github.com>
- Date: Thu, 19 Jan 2017 06:20:39 -0800
- To: heycam/webidl <webidl@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <heycam/webidl/pull/217/review/17461378@github.com>
tobie approved this pull request.
>
Note: Although there is only a single property for an IDL attribute, since
accessor property getters and setters are passed a <emu-val>this</emu-val>
value for the object on which property corresponding to the IDL attribute is
accessed, they are able to expose instance-specific data.
-Note: Note that attempting to assign to a property corresponding to a
+Note: Attempting to assign to a property corresponding to a
Can't/shouldn't we express that normatively?
> + (This will subsequently cause a <emu-val>TypeError</emu-val> in a few steps, if
+ the global object does not implement |target| and [{{LenientThis}}] is not
+ specified.)
+ <!-- https://www.w3.org/Bugs/Public/show_bug.cgi?id=18547#c9 -->
+ 1. Otherwise, set |O| to the <emu-val>this</emu-val> value.
+ 1. If |O| is a [=platform object=], then [=perform a security check=], passing |O|,
+ |attribute|'s [=identifier=], and "getter".
+ 1. If |O| is not a [=platform object=] that implements the interface |target|,
+ then:
+ 1. If |attribute| was specified with the [{{LenientThis}}]
+ [=extended attribute=], then return <emu-val>undefined</emu-val>.
+ 1. Otherwise, [=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
+ 1. Let |R| be the result of performing the actions listed in the
+ description of |attribute| that occur on getting (or those listed in the description
+ of the inherited attribute, if this attribute is declared to
+ [=inherit its getter=]), on |O| if |O| is not <emu-val>null</emu-val>.
I find it hard to parse the meaning of the hanging conditional. I assume it means you carry out the getting in both case, but once it's carried with the |O| as `this` and the other one with `undefined` as `this`?
> + 1. Return |F|.
+
+</div>
+
+<div algorithm>
+
+ The <dfn id="dfn-attribute-setter" export>attribute setter</dfn> is created as follows, given an
+ [=attribute=] |attribute|, an [=interface=] |target|, and a [=Realm=] |realm|:
+
+ 1. If |attribute| is [=read only=] and does not have a
+ [{{LenientThis}}], [{{PutForwards}}] or [{{Replaceable}}] [=extended attribute=], return
+ <emu-val>undefined</emu-val>; there is no [=attribute setter=] function.
+ 1. Assert: |attribute|'s type is not a [=promise type=].
+ 1. Let |steps| be the following series of steps:
+ 1. If no arguments were passed, then
+ 1. Let |V| be the value of the first argument passed.
There's something unclear in the above two steps (either there's no argument or there is one).
> +
+<div algorithm>
+
+ The <dfn id="dfn-attribute-setter" export>attribute setter</dfn> is created as follows, given an
+ [=attribute=] |attribute|, an [=interface=] |target|, and a [=Realm=] |realm|:
+
+ 1. If |attribute| is [=read only=] and does not have a
+ [{{LenientThis}}], [{{PutForwards}}] or [{{Replaceable}}] [=extended attribute=], return
+ <emu-val>undefined</emu-val>; there is no [=attribute setter=] function.
+ 1. Assert: |attribute|'s type is not a [=promise type=].
+ 1. Let |steps| be the following series of steps:
+ 1. If no arguments were passed, then
+ 1. Let |V| be the value of the first argument passed.
+ 1. Let |id| be |attribute|'s [=identifier=].
+ 1. Let |O| be <emu-val>null</emu-val>.
+ 1. If |attribute| is not a [=static attribute=]:
`If |attribute| is a [=regular attribute=]:`
> +<div algorithm>
+
+ The <dfn id="dfn-attribute-setter" export>attribute setter</dfn> is created as follows, given an
+ [=attribute=] |attribute|, an [=interface=] |target|, and a [=Realm=] |realm|:
+
+ 1. If |attribute| is [=read only=] and does not have a
+ [{{LenientThis}}], [{{PutForwards}}] or [{{Replaceable}}] [=extended attribute=], return
+ <emu-val>undefined</emu-val>; there is no [=attribute setter=] function.
+ 1. Assert: |attribute|'s type is not a [=promise type=].
+ 1. Let |steps| be the following series of steps:
+ 1. If no arguments were passed, then
+ 1. Let |V| be the value of the first argument passed.
+ 1. Let |id| be |attribute|'s [=identifier=].
+ 1. Let |O| be <emu-val>null</emu-val>.
+ 1. If |attribute| is not a [=static attribute=]:
+ 1. If the <emu-val>this</emu-val> value is <emu-val>null</emu-val> or
If find all of the substeps of this step 5 super difficult to parse. I don't doubt that the algorithm is correct, but I really struggle to understand its intent. I can't imagine modifying something in here without accidentally breaking stuff.
Here's a proposed alternative (which is slightly less DRY, but hopefully a bit clearer). I added an optional note (prefixed with ?) to clarify the exit point of the substep. Maybe that helps (or is over the top—I'm not sure).
```
1. If |attribute| is a [=regular attribute=]:
1. If the <emu-val>this</emu-val> value is <emu-val>null</emu-val> or
<emu-val>undefined</emu-val>, set |O| to |realm|'s [=Realm/global object=].
(This will subsequently cause a <emu-val>TypeError</emu-val> in a few steps, if
the global object does not implement |target| and [{{LenientThis}}] is not
specified.)
<!-- https://www.w3.org/Bugs/Public/show_bug.cgi?id=18547#c9 -->
1. Otherwise, set |O| to the <emu-val>this</emu-val> value.
1. If |O| is a [=platform object=], then [=perform a security check=], passing |O|,
|id|, and "setter".
1. Let |validThis| be `true` if |O| is a [=platform object=] that
implements the interface |target|, or `false` otherwise.
1. If |validThis| is `true`, then:
1. If |attribute| is declared with a [{{LenientSetter}}] extended attribute, then:
1. Return <emu-val>undefined</emu-val>.
1. If |attribute| is declared with a [{{PutForwards}}] extended attribute, then:
1. Let |Q| be [=?=] [=Get=](|O|, |id|).
1. If [=Type=](|Q|) is not Object, then [=ECMAScript/throw=] a
<emu-val>TypeError</emu-val>.
1. Let |forwardId| be the identifier argument of the [{{PutForwards}}] extended
attribute.
1. Perform [=?=] [=Set=](|Q|, |forwardId|, |V|).
1. Return <emu-val>undefined</emu-val>.
1. If |attribute| is declared with the [{{Replaceable}}] extended attribute, then:
1. Perform [=?=] [=CreateDataProperty=](|O|, |id|, |V|).
1. Return <emu-val>undefined</emu-val>.
? Note: setters who are not special-cased here will
? continue being defined in the parent steps
? along with static attributes.
1. Otherwise if |attribute| was specified with the [{{LenientThis}}] [=extended attribute=],
1. If |attribute| is declared with the [{{Replaceable}}] extended attribute,
then perform [=?=] [=CreateDataProperty=](|O|, |id|, |V|).
1. Return <emu-val>undefined</emu-val>.
1. Otherwise, [=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
```
> + 1. Let |steps| be the following series of steps:
+ 1. If no arguments were passed, then
+ 1. Let |V| be the value of the first argument passed.
+ 1. Let |id| be |attribute|'s [=identifier=].
+ 1. Let |O| be <emu-val>null</emu-val>.
+ 1. If |attribute| is not a [=static attribute=]:
+ 1. If the <emu-val>this</emu-val> value is <emu-val>null</emu-val> or
+ <emu-val>undefined</emu-val>, set |O| to |realm|'s [=Realm/global object=].
+ (This will subsequently cause a <emu-val>TypeError</emu-val> in a few steps, if
+ the global object does not implement |target| and [{{LenientThis}}] is not
+ specified.)
+ <!-- https://www.w3.org/Bugs/Public/show_bug.cgi?id=18547#c9 -->
+ 1. Otherwise, set |O| to the <emu-val>this</emu-val> value.
+ 1. If |O| is a [=platform object=], then [=perform a security check=], passing |O|,
+ |id|, and "setter".
+ 1. Let |validThis| be <emu-val>true</emu-val> if |O| is a [=platform object=] that
Shouldn't the booleans here really be just \`true\` ans \`false\`? These are not JS values, they're infra booleans, no?
>
- The value of the <emu-val>Function</emu-val> object’s “length”
- property is the <emu-val>Number</emu-val> value <emu-val>1</emu-val>.
+ <dl class="switch">
Why are we using a switch here rather than a regular if..otherwise clause?
>
- The value of the <emu-val>Function</emu-val> object’s “name”
- property is a <emu-val>String</emu-val> whose value is the
- concatenation of “set ” and the identifier of the attribute.
+ 1. Perform the actions listed in the description of |attribute| that occur on setting, on
+ |O| if |O| is not <emu-val>null</emu-val>.
Same comments as for the hanging conditional in the getter, here.
--
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/217#pullrequestreview-17461378
Received on Thursday, 19 January 2017 14:21:12 UTC