943958 - Global variables with the same name as window properties can't accessed

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Jason Barnabe (np)]

(Outside of any function)

var name = 1;
alert(typeof name);

Results:

Firefox 27.0a2 (2013-11-27): alerts "number"
Firefox 28.0a1 (2013-11-27): alerts "string"
Chrome 31.0.1650.57: alerts "string"

Not sure if this was intentional, but it broke my extension Stylish.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This was intentional.  See bug 932322.

Comment 2

[Jason Barnabe (np)]

OK. This is easy to fix in my code, but I'm wondering how many others will get hit by this. It's certainly a "WTF" result until you realize what's happening.

-Could we get Firefox to warn when declaring a global variable that's already a property of window?
-Can we get this added to "Firefox 28 for developers"?

Comment 3

[Boris Zbarsky [:bzbarsky]]

I flagged bug 932322 as dev-doc-needed; good idea.

For the other, going to defer to the JS folks...

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: bug-943958-warn-redecl-v1.patch

>-Could we get Firefox to warn when declaring a global variable that's already a property of window?

That seems like a good idea. With this patch I get a warning:

    TypeError: redeclaration of global property name

with file and line number pointing to the declaration. I'd be happier if it didn't say TypeError there. Separate bug for the devtools team, I think.

In the shell, it looks like this:

    js> Object.defineProperty(this, "name", {
            get: function() { return "george"; },
            set: function (x) {}});
    js> var name = "fred";
    typein:2:4 warning: redeclaration of global property name
    js> name
    "george"

What do you think, Jeff? If you like it, I'll figure out how to test it. A new assertWarning() function, combined with indirect eval, or something.

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 8343537 [details] [diff] [review]
bug-943958-warn-redecl-v1.patch

Review of attachment 8343537 [details] [diff] [review]:
-----------------------------------------------------------------

The TypeError bit is an engine bug, really, that we conflate warnings and errors in the same mechanism.  We need to add a separate warning mechanism at some point, to make warnings sane.  Not here, of course.

Comment 6

[Liviu Cirdei]

Is anything else that should be done on this bug, or should we close it?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Well, we could try landing that warning patch....

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.