window itself is both a getter and a base

[jackbsteinberg]

When doing window get/sets, the current pattern sets them up like so:

window.status = 'open';
const n = window.name;
// transforms to
Reflect_apply(Window_status_set, window, ['open']);
const n = Reflect_apply(Window_name_get, window);

This will prove to be an issue once #29 is resolved, since window on its own is a getter. This means window get/sets will need to be treated like this:

window.status = 'open';
const n = window.name;
// transforms to
Window_status_set('open');
const n = Window_name_get();

[fergald]

Why wouldn't it be something like

Reflect_apply(Window_name_get, Window_getter())

?

[domenic]

I'm not sure what Window_getter() would be in that scenario. If you mean that it's the getter for the global window property (which we would normally name Window_window_get), then, why are you making an exception and calling that particular global-property getter with no this value, while you're choosing to call Window_name_get with a this value?

[fergald]

If Window_window_get is the thing that gives you the window object then what I should I have written was

Reflect_apply(Window_name_get, Window_window_get())

I'm trying to reduce the number of special cases and avoid embedding an assumption that there is only one instance of Window in the world.

It seems like window's getters and setters are all going to be special cased with Window_status_set etc not taking a this but still operating on an object instance. It would be nicer if the only special case was for getting that the special instance of Window. Then the getters and setters are no longer special. That original doesn't have to be named Window_window_get (that should probably continue to just exist as something that returns a getter that needs a this). It would be something special since the window we want is the special instance of Window that is injected into the namespace (unlike e.g. document which is just a thing you implicitly get from window).

Also, it's not the case that window is the only instance of Window in the world. What about

const n = frame.contentWindow.name;

This has to become

const n = Window_name_get(frame.contentWindow);

(actually there should proabably be a getter in there for .contentWindow but you know what I mean).

[domenic]

I'm trying to reduce the number of special cases

I think your suggestion actually increases the number of special cases. Instead of aligning with the Web IDL spec, which makes it so that global objects are the ones getting special treatment, it instead treats a specific getter applied to the global object as the special case.

This has to become

Agreed. That will still work, because it's not a bare getter. We can add a test to confirm; I will open a separate issue to confirm.

[fergald]

I think there is a concrete problem with the current approach. Try this

<iframe id=i src="frame.html">
<script>
  let wScrollBy = scrollBy;
  let iScrollBy = i.contentWindow.scrollBy;
  console.log(wScrollBy === iScrollBy);

  let wClose = close;
  let iClose = i.contentWindow.close;
  console.log(wClose === iClose);
</script>

The output is

false
true

Some things on Window are auto-bound to the instance and some are unbound methods. So we can't in general handle globals by just translating them to Window_foo because some of them need to know their instance object.

I'm not sure how to translate above script using the current approach but it seems like we would need for 2 versions of Window_scroll_by one that is already bound to the global and one that isn't.

Is there some other way?

The conceptual point I was trying to make (although I'm actually not sure this is really what's happening under the hood) is that we can think of usage of global variables short-hand for accesses on the global object. I.e

scrollBy(0, 100);

is really

{the global object}.scrollBy(0, 100);

For script running in a window the global object is an instance of Window, for script running in a service worker it's an instance of WorkerGlobalScope(?). So the way to handle these is to provide a getter for the global, maybe Global_get (or maybe Window_get and WorkerGlobalScope_get, I'm just picking one for example) then translate

scrollBy(0, 100);
i.contentWindow.scrollBy(0, 100);

to

Reflect_apply(Window_scroll_by, Global_get(), 0, 100);
Reflect_apply(Window_scroll_by, i.contentWindow, 0, 100);

P.S. If we let's resolve this F2F if it keeps going on.

[domenic]

The output is

This is due to Chrome's buggy implementation of the spec. It won't impact get-originals. The result should be false in both cases. (It is in Firefox at least; I can't test Safari right now.)

some of them need to know their instance object.

Nope. Try calling them. They are not bound, but neither do they need to know their instance object. Instead, they apply to whatever this value they are given, or to the current window if given the undefined this value (e.g. if called as functions).

is really

This isn't correct. Instead, what happens is inside the (Web IDL-generated bindings of the) scrollBy function, there is a special branch, which says that if the this value is undefined, it applies to the global object.

The intention of the current transform is to preserve this, by preserving the this value that it is applied to. In particular, scrollBy(0, 100) is applied to an undefined this value. We preserve that by translating it to Window_scrollBy(0, 100), which is another function call with undefined this value. (We could redundantly state the undefined this value, with Reflect_apply(Window_scrollBy, undefined, [0, 100]), but this is confusing and verbose, effectively turning a function call into a method call on an undefined base.)

Similarly, when a this value is supplied, as in the example contentWindow.scrollBy(0, 100), where the this value is contentWindow, we should preserve the this value again, by translating it to Reflect_apply(Window_scrollBy, contentWindow, [0, 100]).

So as I said, I think we should add a test to make sure that latter case is translated correctly; by code inspection it should fall out correctly, but you never know.

But it is not correct to try to change an undefined this value into a "get the global" this value. This has slight observable consequences, in particular because there is no way (for security reasons, I believe) to get access to the actual global object; you can only indirectly call methods on it in the above fashion. (The value returned by self or window is the global proxy; see https://blog.whatwg.org/windowproxy-window-and-location or https://mathiasbynens.be/notes/globalthis#terminology.)

[fergald]

OK. Thanks for the explanation. Is it worth filing/fixing bugs?

[domenic]

Is it worth filing/fixing bugs?

I ran the test again and it appears to output false in both cases in Canary. So it looks like this was fixed :). http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=7176

I think it is related to https://bugs.chromium.org/p/chromium/issues/detail?id=761755 but I don't see mentions of this specific scenario (function properties) in that bug or any of the ones it blocks.

I'll close this and file a new issue for adding tests that exercise separate windows.