Issue 1287 - Use window.location instead document.location

Issue 1287 - Use window.location instead document.location

[Wladimir Palant, 2014-08-29 19:54:28]

I generally prefer spelling things out: window.location.href (to make it obvious
that location is a predefined window property rather than a global variable). I
think we actually do that fairly consistently. If you decide that you don't want
to fix this - LGTM regardless.

[Sebastian Noack, 2014-08-29 19:59:11]

I don't have a strong opinion on that. However I like consistency. And we don't
spell out window.document, window.Array, window.Object, etc. So why
window.location?

[Wladimir Palant, 2014-08-29 20:44:37]

On 2014/08/29 19:59:11, Sebastian Noack wrote:
> I don't have a strong opinion on that. However I like consistency. And we
don't
> spell out window.document, window.Array, window.Object, etc. So why
> window.location?

The reason I got into this habit was actually a move that happened in our
Firefox code a while ago - it used to run in a window context, and then it no
longer did. So window became a parameter and each unqualified setTimeout() call
had to be tracked down.

We do spell out window.setTimeout(), window.arguments, window.navigator,
window.addEventListener() and others. Array, Object & co. are actually global
variables defined by the JavaScript engine on any global object and not specific
to the window object.

From what I can see, window.document is the only special case we currently have
- we indeed don't spell out this one.

[Sebastian Noack, 2014-08-29 21:55:51]

On 2014/08/29 20:44:37, Wladimir Palant wrote:
> On 2014/08/29 19:59:11, Sebastian Noack wrote:
> > I don't have a strong opinion on that. However I like consistency. And we
> don't
> > spell out window.document, window.Array, window.Object, etc. So why
> > window.location?
> 
> The reason I got into this habit was actually a move that happened in our
> Firefox code a while ago - it used to run in a window context, and then it no
> longer did. So window became a parameter and each unqualified setTimeout()
call
> had to be tracked down.
> 
> We do spell out window.setTimeout(), window.arguments, window.navigator,
> window.addEventListener() and others. Array, Object & co. are actually global
> variables defined by the JavaScript engine on any global object and not
specific
> to the window object.
> 
> From what I can see, window.document is the only special case we currently
have
> - we indeed don't spell out this one.

I see. However, the problem you try to address seems to be Gecko specific. And
in fact, fully-qualifying global variables, by prepending "window.", is a rather
uncommon practice in the Chrome/OPera/Safari codebase.

[Wladimir Palant, 2014-08-29 22:20:52]

On 2014/08/29 21:55:51, Sebastian Noack wrote:
> I see. However, the problem you try to address seems to be Gecko specific.

Really? And what if some of the code you write there moves to libadblockplus
tomorrow?

> And
> in fact, fully-qualifying global variables, by prepending "window.", is a
rather
> uncommon practice in the Chrome/OPera/Safari codebase.

I agree, it seems to be a bit inconsistent. As I said, it is up to you whether
you want to address this.