110397 - innerHeight and innerWidth pollute the window scope

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Judson Valeski]

This is causing problems w/ http://www.intelihealth.com.

Comment 1

[Brendan Eich [:brendan]]

Jud, thanks -- jst is already on the case.

/be

Comment 2

[Johnny Stenback (:jst)]

Attachment: Make a bunch of writable properties replaceable.

Comment 3

[Johnny Stenback (:jst)]

The attached patch makes innerHeight, innerWidth, outerWidth, outerWidth,
screenX, screenY, status, and name "replaceable" if assigned to w/o fully
qualifying the name (i.e. |window.name = "foo";| will set the window name, |var
name = "foo"| (with or w/o the |var|) will not set the window name).

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 58095 [details] [diff] [review]
Make a bunch of writable properties replaceable.

r=peterv.

Comment 5

[Fabian Guisset]

Is it possible to do this for location as well? or is it harder because it is
already coded in DOMClassInfo? we have a couple of bugs about var location not
shadowing window.location.

Comment 6

[Dan M]

  Won't this cause backward compatibility problems? Since to window. or not to
window. has historically been equivalent, websites have been uncareful about
which they used. This patch draws a distinction; it makes them no longer
equivalent. Now "window.innerHeight=200" will change the window's size, but
"innerHeight=200" won't. This will happen to prevent intellihealth's JS from
tripping over Mozilla but I'm betting it will surprise other sites. With the
distinction having historically been unimportant, the web is full of examples of
all possible styles of usage. I imagine.
  I agree that the new distinction is logical. And it probably won't cause very
many problems because most websites wouldn't use innerHeight to size their
windows, since it wouldn't work on IE. Hmm. I guess I like the patch because I
think it'll do more good than harm. But I am concerned what it means to begin
drawing a distinction between using the window. qualifier or not. From David
Flanagan's JavaScript book: "In some cases, it is useful to use [window. or
self.] to make your code clearer or to disambiguate it. Using these properties
is largely a stylistic matter, however."

Comment 7

[Johnny Stenback (:jst)]

Yup, the big question here is "What's worse, what we have or what we'll get?". I
don't know the answer, but I suspect that this patch would fix more sites that
it would break. The only possible way to find out, IMO, is to check it in and
see how it goes (i.e. leave it in for a milestone or so).

Fabian, I don't want to do this for location since I know I've seen people do:

  location = "http://...";

and that does work in IE and NS4x and Mozilla, if we'd make this change for
location as well the above wouldn't work in mozilla any more and location is
kinda frequently used so I say no :-)

Comment 8

[Johnny Stenback (:jst)]

I talked to Vidur about this and his thought's about this are the same as mine,
it's probably ok to do this for the properties mentioned above, but we'd better
leave location alone.

Brendan, Jband, whaddaya think?

Comment 9

[Brendan Eich [:brendan]]

The location botches must be because the 4.x or earlier JS engine, upon seeing
'var location ...', would redefine the built-in.  That's broken per ECMA and I
can't believe that IE does it.  So I think anyone who wants var location to
shadow the predefined property is SOL.

sr=[e-mail].

/be

Comment 10

[Johnny Stenback (:jst)]

Fix checked in, fingers crossed in hope that this actually helps more than it
breaks :-)

Comment 11

[Johnny Stenback (:jst)]

*** Bug 138303 has been marked as a duplicate of this bug. ***

Comment 12

[Joaquin Menchaca]

Hi.

I'm trying to understand this bug, and why this fix was necessary to break DOM
Level 0 compatibility.  I am sure there are good reasons, but there needs to be
some documentation or something.

It's frustrating because code that is documented on just about every textbook
out there, including David Flannagan's JavaScript the Definitive Guide, uses the
vanilla without the window.  This also breaks scripts that onced worked with 3rd
Gen browsers and Internet Explorer upto the current version 6.