We fail http://test262.ecmascript.org/ test S15.10.6.2_A1_T16. /blah/.match() should be the same as /blah/.match("undefined"). Instead we throw a SyntaxError. As far as I can tell from the ES5 spec, the test is right and we're wrong. http://people.mozilla.org/~jorendorff/es5.html#sec-15.10.6.2

We fail S15.10.6.3_A1_T16 for the same reason.

RegExps have no match method -- do you mean exec? We do the same as Chrome and Safari on /blah/.exec(). /be

Oops. I meant /blah/.exec(). Also /blah/.test(). IE9 and Chrome both get this right. Safari throws an Error rather than a SyntaxError. Here are the tests: Test S15.10.6.2_A1_T16 Description RegExp is /undefined/ and call exec() without arguments Testcase function testcase() { __re = /undefined/.exec()[0]; if (__re !== "undefined") { $ERROR("#1: /undefined/.exec()[0] === \"undefined\". Actual: " + __re); } } Test S15.10.6.3_A1_T16 Description RegExp is /undefined/ and call test() without arguments Testcase function testcase() { __re = /undefined/; if (__re.test() !== (__re.exec() !== null)) { $ERROR("#0: __re = /undefined/; __re.test() === (__re.exec() !== null)"); } }

I would need to remove this non-standard feature, that calling test/exec with no arguments uses the previous input. Otherwise we could come up with cases where we either match 'undefined' or on the previous input. From what i see this isn't documented, and fwiw i didn't even know we had such an extension.

test262 is hitting this? I think that means we fix, and remove the old stuff if necessary.

Yes. Summary: String.prototype.test(regexp) String.prototype.match(regexp) Does new RegExp(regexp), so regexp of undefined => "" (empty string) |this| of undefined would throw String.prototype.split(separator, limit) Undefined separator just returns [|this|] String.prototype.replace(searchValue, replaceValue) If search value isn't a regexp, it's just ToString(searchValue), so 'undefined' new RegExp(pattern, flags) undefined pattern => "" Regexp.prototype.match(string) Regexp.prototype.test(string) Just ToString(string), so "undefined"

Sorry made some mistakes. Summary: String.prototype.search(regexp) String.prototype.match(regexp) Does new RegExp(regexp), so regexp of undefined => "" (empty string) |this| of undefined would throw String.prototype.split(separator, limit) Undefined separator just returns [|this|] String.prototype.replace(searchValue, replaceValue) If search value isn't a regexp, it's just ToString(searchValue), so 'undefined' new RegExp(pattern, flags) undefined pattern => "" RegExp.prototype.exec(string) RegExp.prototype.test(string) Just ToString(string), so "undefined"

Comment on attachment 531669 [details] [diff] [review] remove ancient behavior and make regexp functions ES5 compliant Review of attachment 531669 [details] [diff] [review]: ----------------------------------------------------------------- Drive-by mini-review comment... ::: js/src/jsregexp.cpp @@ +650,4 @@ > input = js_ValueToString(cx, vp[2]); > if (!input) > return false; > vp[2] = StringValue(input); This can be shortened a little bit. Start like this: JSString *input = js_ValueToString(cx, argc > 0 ? vp[2] : UndefinedValue()); Don't bother assigning input back to vp[2]. Then change RegExp::executeInternal to anchor input just before getting input->chars(), around line 347 of jsregexpinlines.h.

Here's the code I meant to quote. 643 649 /* Step 2. */ 644 /* Step 2. */ 650 JSString *input; 645 JSString *input; 651 if (argc > 0) { 646 if (argc == 0) { 647 input = cx->runtime->atomState.typeAtoms[JSTYPE_VOID]; 648 } 649 else { 652 input = js_ValueToString(cx, vp[2]); 650 input = js_ValueToString(cx, vp[2]); 653 if (!input) 651 if (!input) 654 return false; 652 return false; 655 vp[2] = StringValue(input); 653 vp[2] = StringValue(input);

I love Bugzilla.

So i would have done this a lot earlier, but i remembered that, i said you should stop reviewing, because of something i wanted to change, but i just can't figure out what it was. So i tried to get into the code again and except some null check, i didn't find anything. This scares me a bit :/

Comment on attachment 546863 [details] [diff] [review] v2 Review of attachment 546863 [details] [diff] [review]: ----------------------------------------------------------------- Looks good with one minor quibble. I'll change it and push. ::: js/src/jsregexpinlines.h @@ +346,3 @@ > JSLinearString *input = inputstr->ensureLinear(cx); > if (!input) > return false; This anchors the wrong thing. Since you're going to call input->chars(), you need to anchor input, not inputstr. The purpose of the anchor is to protect those chars from GC.

evilpie updated the docs and I did some cleanup: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/RegExp/exec https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/RegExp/test https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/String/search https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/String/match And listed on Firefox 8 for developers.

I believe it's just typo but not 100% sure and let me ask here: (In reply to Eric Shepherd [:sheppy] from comment #16) > evilpie updated the docs and I did some cleanup: > > https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/RegExp/ > exec > https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/RegExp/ > test You wrote: ... it would match against the value of the RegExp *property* input ... But it should be ... it would match against the value of the RegExp *previous* input ... right?

Well previous input == input property.

Ahh, you mean the deprecated RegExp.input property. https://developer.mozilla.org/en/JavaScript/Reference/Deprecated_Features Thanks and I've updated the documents clearer: ... match against the value of the previous input (RegExp.input property) ...