"Cross-origin objects" are to have enumerable properties - #6538

Do we still want to keep those checks in the test?

/*

• [[Enumerate]]
  */

addTest(function() {
for (var prop in C)
assert_unreached("Shouldn't have been able to enumerate " + prop + " on cross-origin Window");
for (var prop in C.location)
assert_unreached("Shouldn't have been able to enumerate " + prop + " on cross-origin Location");
}, "[[Enumerate]] should return an empty iterator");

Claiming the properties are enumerable but then not being able to see them when enumerating seems broken to me?

Sigh. I wish I had spotted that before. https://tc39.github.io/ecma262/#sec-enumerate-object-properties uses [[OwnPropertyKeys]] ([[Enumerate]] is no more it seems) so named properties will still be hidden, but generally everything else should indeed enumerate. I'll write a patch.

See #6571 for the follow-up.

Shouldn't there be tests for the behavior on non-cross-origin objects too? Or are there already?

I think same-origin is tested by html/dom/interfaces.html already.

I think same-origin is tested by html/dom/interfaces.html already.

Does that test the indexed props?

Adding a test for that now in the other PR. It seems that named properties are still non-enumerable in the same-origin case due to the named properties object setup and Window having LegacyUnenumerableNamedProperties.

(And FTR, that's tested already by html/browsers/the-window-object/window-named-properties.html.)

It seems that named properties are still non-enumerable in the same-origin case due to the named properties object setup and Window having LegacyUnenumerableNamedProperties.

Uhhh I see... Maybe the consistency argument is not so strong for switching "named properties" (i.e. those from the document-tree child browsing context name property set) to enumerable for cross-origin then? :-/.

@domenic given that cross-origin they're on a totally different object I think it's fine. We could maybe consider changing the same-origin setup, but maybe it's best to just leave it alone.

Well, but whatwg/html#2777 intent was

It makes all cross-origin properties that would normally be enumerable on same-origin objects, enumerable also on WindowProxy and Location objects (including when accessed same-origin).

Before that the spec had them as non-enumerable (i.e. same-origin WindowProxy and cross-origin WindowProxy both exposed them as non-enumerable). So maybe we should revert that part of the spec?

Well in one case they're on WindowProxy and in the other case they're on some prototype object (the named properties object). Looking at the Gecko patch it's probably easier if all cross-origin properties are enumerable consistently, though @bzbarsky can correct me.

In Gecko it's easy either way. The patch in Gecko just removes code that forced all cross-origin properties to be non-enumerable. The code that puts the named props on the cross-origin window is at http://searchfox.org/mozilla-central/rev/a83a4b68974aecaaacdf25145420e0fe97b7aa22/js/xpconnect/wrappers/XrayWrapper.cpp#2059-2076 and it would be quite simple to have it pass false for the optional fifth argument (enumerable) to FillPropertyDescriptor.

I am a bit confused here. I understand we expose named properties on cross-origin windows (i.e. we allow looking up iframe by name). However, those frame names are NOT exposed cross-origin, are they? I believe doing so would leak cross-origin information and I thought this is why we had [[HideFromKeys]]: true for those crossOriginWindowProperties in the spec. If this is true, I don't think they should be enumerable either.

The set of frame names is not exposed cross-origin. That is, [[OwnPropertyKeys]] does not include them in its return value.

If you know a specific frame name, you can get the property.

Since the names are not returned by [[OwnPropertyKeys]], they're cannot be enumerated viaa for...in. So the only thing affected by whether they're enumerable is what getOwnPropertyDescriptor returns, afaict.

I'm happy with the specification as it stands now, but if folks feel strongly I can make more changes.

@bzbarsky Ok, this was my understanding. As a result, I don't think getOwnPropertyDescriptor() should lie and claim those properties are enumerable (as they are actually not).

It's not lying. It's just that "enumerable" in getOwnPropertyDescriptor is totally unrelated to whether for..in sees the property, if [[OwnPropertyKeys]] lies about whether the property exists at all...

Anyway, I don't have a very strong feeling about the named props issue, because it doesn't have any real practical impact; happy to implement whatever people decide on there.

I'm happy to stop churning on this and call it done.

Your call. I feel it would be cleaner for enumerable to be false but in the end, it does not really matter since there is no behavior difference.