Bug 13790 – Function declarations are not treated as statements (used to affect starcraft2.com)

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Darin Adler]

I was having problems at the new Starcraft II site <http://www.starcraft2.com>.

I narrowed it down to a problem in <http://www.starcraft2.com/js/tooltip.js>, which is a modified copy of the Dynamic Drive DHTML tooltip script. Here's an excerpt:

    if (ie||ns6)
    //var tipobj =  document.getElementById("dhtmltooltip");

    function ietruebody(){

This boils down to "if (x) function y()", which is a syntax error in our implementation, but works fine in Firefox at least. I didn't test IE, but I'm pretty sure it works fine there.

This is a case where matching the ECMAScript standard is causing compatibility problems.

[Darin Adler]

<rdar://problem/5216063>

[David Kilzer (:ddkilzer)]

Another Starcraft II bug:  Bug 13787.  Not sure if they're related or not.

[Robbert Broersma]

Created attachment 19187 [details]
JavaScript file that causes a syntax error unexpectedly using an inline defined named function

I have attached an example where the SyntaxError occurs. This is a weird ECMAScript implementation issue. The script doesn't cause errors in IE7 and FF3.

[Cameron Zwarich (cpst)]

If you look at http://www.starcraft2.com/js/tooltip.js, you will notice that the 'if' is also commented. It seems that they did not mean to comment only the var statement. The original source for that JS library also seems to support this: http://www.dynamicdrive.com/dynamicindex5/dhtmltooltip.htm.

Can we close this bug?

[Darin Adler]

If we understand that we're different from the other JavaScript engines (with higher market share) and our considered decision is that we don't want to match their behavior, then sure, we can close the bug.

I'm sure others will make this mistake again in the future on other websites. But maybe it's OK if we behave differently.

[Cameron Zwarich (cpst)]

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

[Cameron Zwarich (cpst)]

I will probably just do this. It will avoid future compatibility issues and it will even simplify our grammar.

[Cameron Zwarich (cpst)]

Created attachment 25467 [details]
Proposed patch (without test)

Here is a patch that fixes this problem. I will add some tests for this new behaviour and put it up for review.

[Cameron Zwarich (cpst)]

Created attachment 25474 [details]
Proposed patch

[Geoffrey Garen]

Comment on attachment 25474 [details]
Proposed patch

r=me

JavaScript stinks.

[Cameron Zwarich (cpst)]

Landed in r38747.