Check ignoreNamedProps before the named property visibility algorithm. #625
Conversation
|
This assumes that we all agree that "and" short-circuits; I'm not actually sure that's well-defined. |
|
Funtimes. I think it might be clearer to just nest:
Either way, we should probably have tests. |
|
Fixed. |
index.bs
Outdated
| 1. Set |desc|.\[[Configurable]] to <emu-val>true</emu-val>. | ||
| 1. Return |desc|. | ||
| 1. If |O| [=support named properties|supports named properties=] and | ||
| |ignoreNamedProps| is false: |
There was a problem hiding this comment.
Usually if statements are followed by: ", then:". Or does IDL use a different style?
There was a problem hiding this comment.
I'd like this to be addressed, given that we do use then on the subsequent line. Perhaps some kind of editorial issue needs to be filed to make sure we're consistent.
(As far as I can tell this PR is otherwise okay.)
|
Can you test this in some non-Gecko browsers as well? |
|
@littledan It's worth testing in Safari. Testing in Chrome... given #607 (comment) Chrome is not even trying to follow the spec here, so I doubt testing in Chrome will tell us much, but we could try. Interpreting the results of black-box tests is pretty complicated when you have no idea what the underlying model might look like. |
|
I haven't yet been able to observe the named property visibility algorithm in either Chrome or WebKit, which makes this change unobservable there. Test case<!DOCTYPE html>
<script>
sessionStorage.clear();
sessionStorage.__proto__ = new Proxy(Storage.prototype, {
has(_, p) { w("has " + p); return true; },
get(_, p) { w("get " + p); return "from proto" },
});
w("===========");
w(sessionStorage.x);
w("===========");
sessionStorage.x = "foo";
w("===========");
w(sessionStorage.x);
w("===========");
</script>Chrome and WebKit: Gecko: |
|
I'm a little confused by that testcase, actually. Shouldn't it use a proxy for "localStorage" or "sessionStorage" or something, not "Storage.prototype"? There should be no named property visibility stuff involved with "Storage.prototype". |
|
I'm more than a little confused by it myself now; looks like I somehow merged two different test cases. Should be better now, and still shows that the change isn't observable in Chrome/WebKit. |
|
For clarity, #625 (comment) remains outstanding here. |
Running the named property visibility algorithm is (or should be) observable by inserting a proxy in the prototype chain. This changes the spec to match Gecko's behavior.
Running the named property visibility algorithm is (or should be) observable
by inserting a proxy in the prototype chain. This changes the spec to match
Gecko's behavior.
Preview | Diff