388643 - JavaScript Tests - global shell.js sets JavaScript version to version lower than required by E4X tests

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Caldwell]

Attachment: Sets version to '160' in e4x/shell.js

The global shell.js currently sets the JavaScript version to 1.5.  I'm not clear on why this is, but it causes all E4X tests to fail in Rhino because Rhino only enables E4X when version >= 1.6.

The patch I will attach simply modifies the e4x/shell.js to declare the version to be 1.6.  But I wonder whether Rhino's 1.7+ features will break also as a result of this change.  So attempting to CC Norris as well.

Comment 1

[Norris Boyd]

There's no reason I can think of offhand for the global shell.js to set the version to 1.5. Where is that code? I searched for "150" through the codebase and didn't see it.

Comment 2

[David Caldwell]

See line 47 at http://lxr.mozilla.org/mozilla/source/js/tests/shell.js.  It does have a comment, but I'm not tracking SpiderMonkey enough to interpret it.

// Spidermonkey shell now defaults to 1.8, so set the basic version to
// 1.5 for backwards compatibility.

if (typeof version != 'undefined')
{
  version(150);
}

Comment 3

[Bob Clary [:bc] (inactive)]

The new default of version 1.8 in SpiderMonkey broke a number of older tests. Since versions 1.0-1.4 are irrelevant now, I set the overall default to 1.5 in the global shell.js. e4x support is controlled via options in SpiderMonkey and not solely by version. I think this changing of the default version for e4x to 1.6 will be ok.

Comment 4

[Norris Boyd]

I didn't see that change as I hadn't synced to the latest version of shell.js. 

Comment 5

[Norris Boyd]

I've run the Rhino tests with David's patch (thanks, David!) and cleaned up a bunch of other issues. I now only get one failure in js1_5/Regress/regress-309242.js, which is a test that uses E4X but is not in the e4x directory. Any ideas for what to do here? 

Comment 6

[Bob Clary [:bc] (inactive)]

If the comment hack is something you want to test, we can create a dupe in the js1_6 suite which should enable e4x for you and add the js1_5 version to your exclusion list. Otherwise just exclude it altogether?

Comment 7

[Norris Boyd]

Perhaps it's easiest to just add js1_5/Regress/regress-309242.js to the Rhino skip list. 

Bob, can David or I commit David's patch in this bug? It's working well for me.

David, perhaps we should consider enabling E4X on all JavaScript versions for 1.7R1 release if that's what SpiderMonkey does.

Comment 8

[Bob Clary [:bc] (inactive)]

(In reply to comment #7)

> Bob, can David or I commit David's patch in this bug? It's working well for me.

sure, go ahead.

Comment 9

[Norris Boyd]

Fixed:

Checking in e4x/shell.js;
/cvsroot/mozilla/js/tests/e4x/shell.js,v  <--  shell.js
new revision: 1.23; previous revision: 1.22
done