Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Reject' definition. #73

Closed
ylafon opened this issue Nov 16, 2015 · 6 comments
Closed

'Reject' definition. #73

ylafon opened this issue Nov 16, 2015 · 6 comments
Labels
☕☕ difficulty:medium Hard to fix ⌛ duration:short Should be a short fix

Comments

@ylafon
Copy link
Collaborator

ylafon commented Nov 16, 2015

In https://heycam.github.io/webidl/#named-properties-object-defineownproperty and https://heycam.github.io/webidl/#defineownproperty
The term "Reject" is referring to a non-existent section.
Ex:

...MUST behave as follows when called with object O and property name P. The term “Reject” is used as defined in section @@ @@.

  1. Reject.
@bzbarsky
Copy link
Collaborator

Oh, this went away with platform array objects, looks like. That's bad.

@bzbarsky
Copy link
Collaborator

In particular, in #52

@bzbarsky
Copy link
Collaborator

Oh, but more to the point, we don't need this thing because in ES6 the contract for [[DefineOwnProperty]] doesn't involve throwing at all. You just return false and the caller throws if needed.

@heycam should we just update all out [[DefineOwnProperty]] to the ES6 definition?

@heycam
Copy link
Collaborator

heycam commented Nov 17, 2015

Yes I think we should update. And to me it looks like all [[DefineOwnProperty]] calls in Web IDL currently can just ignore its return value. Do you agree? Apart from http://heycam.github.io/webidl/#defineownproperty which needs to just return it.

@bzbarsky
Copy link
Collaborator

@heycam The calls in dictionary and sequence idl-to-es conversion should never return false. Same thing in the unscopeables setup and serializer bits, So ignoring the return value there makes sense.

The call for [Replaceable] attribute setters, however, I think should probably throw if false is returned. Testing this in browsers is a bit of a pain because you can't freeze a Window effectively, but this is a testcase:

var str = "Object.freeze(self); self.console = 5; postMessage(typeof console);"
var blob = new Blob([str]);
var url = URL.createObjectURL(blob);
var w = new Worker(url);
w.onmessage = function (e) {
  alert("OK: " + e.data);
}
w.onerror = function (e) {
  alert("Error: " + e.message);
}

Looks to me like this throws in Firefox but not Chrome (which silently ignores the set instead)... Chrome doesn't throw with use strict there either, of course. In any case, I think we basically want the behavior here to be like Object.defineProperty, which would throw in this situation, so throwing makes sense to me.

@tobie tobie added bug ⌛ duration:short Should be a short fix ☕☕ difficulty:medium Hard to fix labels Jun 19, 2016
@ylafon
Copy link
Collaborator Author

ylafon commented Jun 22, 2016

Note that the this was fixed by 8c9b828

@ylafon ylafon closed this as completed Jun 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☕☕ difficulty:medium Hard to fix ⌛ duration:short Should be a short fix
Development

No branches or pull requests

4 participants