- From: Domenic Denicola via GitHub <sysbot+gh@w3.org>
- Date: Thu, 16 Jun 2016 12:43:26 +0000
- To: public-houdini-archive@w3.org
domenic has just created a new issue for https://github.com/w3c/css-houdini-drafts: == [css-paint-api] JavaScript spec usage notes and suggestions == ## inputProperties It would be better to accept any iterable for inputProperties, and convert it to a `sequence<DOMString>`, instead of type testing for an array and type testing each element against a string. This fits better with the rest of the platform. To see how to do this, see step 14.9 of https://html.spec.whatwg.org/multipage/scripting.html#element-definition. Right now you iterate over the list imprecisely, using "For each item in inputProperties". This is pretty bad since it raises the question of e.g. does this trigger a getter installed for the property named "0"? ## type testing for paint() I think it would be better to store the paint callback once, instead of re-Get()ing it every time you call it (indirectly via Invoke()). Either that, or you shouldn't bother type-testing at registration time. Right now this passes the type test: ``` let i = 0; class C { get paint() { if (++i > 1) { return "not a function"; } return function () { }; } } ``` ## Calling scripts Before/after calling any scripts manually, you need to prepare to run a script and prepare to run a callback. See https://heycam.github.io/webidl/#es-invoking-callback-functions for an example. This correctly sets up the incumbent and entry settings objects so that if e.g. getting the paint() method executes postMessage, it gets the current incumbent settings object. (This is potentially important for security.) There are two places in the spec that you call into user code: when you pull stuff off the class, and when you invoke paint(). The easiest way to fix this for invoking paint() is to instead convert the paint() callback to a Web IDL callback type at the time you pull it off the class. That means later when you invoke it, you can use Web IDL, which will take care of this for you. However at the time you're pulling stuff off the class, you'll need to do it manually. "Prepare to run script" is easy; just "Prepare to run script with the current settings object" (as custom elements do). However I haven't quite figured out what the correct settings object is for "prepare to call a callback" :(. I'll let you know if I figure it out... ## Editorial Please consider no longer using the "named arguments" notation for abstract operations... it is unlike the rest of the platform and hard to read. Invoke is missing a link Lists in ES use « », not []. Lists are distinct from arrays. Consider using ordered pair notation (x, y) instead of (x - y). Alternately, consider using the map verbiage we are using in other places in the platform, which does not represent a map as a set of ordered pairs but instead talks about entries, keys, and values. See e.g. https://html.spec.whatwg.org/#fetching-scripts:module-map Link to https://tc39.github.io/ecma262/ which has many fixes compared to the ecma-international.org version. Please view or discuss this issue at https://github.com/w3c/css-houdini-drafts/issues/222 using your GitHub account
Received on Thursday, 16 June 2016 12:43:28 UTC