bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: David Caldwell, Unassigned)

Tracking

unspecified
x86
Windows XP
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
Created attachment 272816 [details] [diff] [review]
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

11 years ago
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.
(Reporter)

Comment 2

11 years ago
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

11 years ago
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

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

Comment 5

11 years ago
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

11 years ago
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

11 years ago
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

11 years ago
(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

11 years ago
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
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.