new RegExp(undefined) should be treated like new RegExp(""), per ECMA 5 section 15.10.4.1: "...Otherwise, let P be the empty String if pattern is undefined and ToString(pattern) otherwise, and let F be the empty String if flags is undefined and ToString(flags) otherwise." Instead we return /undefined/.

We flunk several http://test262.ecmascript.org/ tests because of this: S15.10.4.1_A3_T2 S15.10.4.1_A3_T3 S15.10.4.1_A3_T4 S15.10.4.1_A3_T5 S15.10.4.1_A4_T2 S15.10.4.1_A4_T3 Seriously this is stupid. It's like somebody with not much imagination spent all morning trying to think of all the ways to say "undefined".

Is the test being gamed to score us lower than IE9? Serious question, because undefined is undefined, however you spell it. Ok, never attribute to malice... /be

I think these tests are from Sputnik...

Yeah, it's a Sputnik test. It was probably written by someone at Google poring over the spec with a fine-toothed comb. There's nothing wrong with that, and we should converge on what the spec says, even to the point of analities like these. It just means the test results aren't something that should be used in marketing, or should be too important for compliance with the world people generally live in. I may push back against that in a blog post soon.

This transforms new RegExp(undefined) into new RegExp("") like ECMA suggests. (This patch doesn't alter the behavior when the flag is 'undefined', because I'm not sure what the current behavior is, as well as what the behavior should be.)

I noticed I didn't cover everything right, so I wrote a small testcase to make sure I solved this bug right. (I'm ok with this getting included to the "tests", but can somebody point me to a document how to write a decent test?)

Updated the patch. Only problem with this patch is that it doesn't alter the behavior of /undefined/. I think this regexp is made with some other code. Not sure where I can find that code. Also I'm not sure if the behavior of /undefined/ should get altered?? new RegExp(undefined, ...) and RegExp(undefined, ...) work now according to the ECMA spec)

Well I mistakenly thought that /undefined/ equals RegExp(undefined). But it actually equals RegExp("undefined"). Therefor I had a wrong test. So I removed those faulty tests and now I can confirm the patch is finished

Comment on attachment 513881 [details] [diff] [review] Patch r+ with style nits. Thanks for the patch! + if(sourceValue.isUndefined()) { + /* + * per ECMA 5 section 15.10.4.1: + * new RegExp(undefined) should be treated like new RegExp("") + */ Space after if, can probably just cite ECMA on a single line without explanation.

This seems like something we want to get into a dot release if those tests are high profile.

Comment on attachment 513881 [details] [diff] [review] Patch Agree on the nits (Chris, we often pick them for the contibutor instead of requiring a new patch be attached). The bug isn't a blocker but the patch is obviously safe for next release, which is still Fx4, not 4.x. I say we go for patch approval. /be

Has approval. Not blocking.

(In reply to comment #8) > Created attachment 513898 [details] haytjes, do I have your permission to release this into the public domain?

(In reply to comment #13) > (In reply to comment #8) > > Created attachment 513898 [details] > > haytjes, do I have your permission to release this into the public domain? Yeah, go ahead ;-)