225343 - Regexp pattern /[A]/i not matching 'a'

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Rob Allen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031110 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031110 Firebird/0.7+

http://www.the-allens.net/csstest/jstest.htm contains a simple form with a JS
email checker. The core code is:

function checkemail(str) {
    var result = false;
    var filter =
/^[-!#$%&\'*+\.\/0-9=?A-Z^_`{|}~]+@([-0-9A-Z]+\.)+([0-9A-Z]){2,4}$/i;

    var test = filter.test(str);
    if (test === true){
        result = true;
    }

    return result;
}


This function fails to correctly identify the email address "[e-mail]" as
valid. IE, Moxilla 1.5 and Opera all successfully identify that email address as
correct.

Reproducible: Always

Steps to Reproduce:
1. Navigate to http://www.the-allens.net/csstest/jstest.htm
2. Press the "update" button
3. Read the JS Application message that is displayed in a popup box.

Actual Results:  
Text in alert() box is "Please enter a valid email address"

Expected Results:  
Text in alert() box should be "Email address is valid"

doron built the seamonkey code today to test this for me and reported that the
error also occurs on seamonkey.

Also, this is a regression from Mozilla 1.5. I haven't tested all nightlys from
then to now as I'm on dial-up.

Comment 1

[Olivier Cahagne]

could be a regression from bug 223273.

Comment 2

[Phil Schwartau]

I have found that if you replace the A-Z character ranges with a-z
in the above example, the bug goes away. That means it reduces to:

js> 'a'.match(/[a]/i);
a                      <----------------------- correct

js> 'a'.match(/[A]/i);
null                   <----------------------- bug


We used to return 'a' for both, but this has changed since the
checkin for bug 85721.

Comment 3

[Phil Schwartau]

Testcase added to JS testsuite:

      mozilla/js/tests/ecma_3/RegExp/regress-225343.js


Currently failing in Rhino as well as SpiderMonkey. Passes if run
against the RC5a release of SpiderMonkey.

I have filed bug 225366 against Rhino for this same issue -

Comment 4

[Brendan Eich [:brendan]]

Phil: can you test ranges also?

/[A-Z]/i('a')

Also, test a bunch of inputs other than 'a' -- test toward the end of the ASCII
charset, test higher into Unicode, etc.

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: proposed fix

I'm not happy with the bugs I'm finding in jsregexp.c.	Hope this is it for
now.

/be

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 135748 [details] [diff] [review]
proposed fix

Sure looks better to me.

Comment 7

[Brendan Eich [:brendan]]

Fixed.  Fixes the checkemail() test function in comment 0.

Phil, the test suite passes except for the known Array truncation performance
test, and these two tests:

          Testcase   ecma_2/RegExp/properties-001.js  failed  Bug  Number
          http://scopus/bugsplat/show_bug.cgi?id=346000
          [ Next Failure | Top of Page ]
          Failure messages were:
          /(?:)/.toString() = /(?:)/ FAILED! expected: //

          Testcase js1_2/regexp/toString.js failed
          [ Previous Failure | Next Failure | Top of Page ]
          Failure messages were:
          var re = new RegExp(); re.toString() = /(?:)/ FAILED! expected: //

Can you update the testsuite to expect empty regexps to decompile as /(?:)/ in
all cases?  Thanks,

/be

Comment 8

[Phil Schwartau]

Marking Verified FIXED. 
Testcase mozilla/js/tests/ecma_3/RegExp/regress-225343.js now passes.


> Fixes the checkemail() test function in comment 0.
True, and this is part of the above testcase, as well 


> Can you update the testsuite to expect empty regexps to decompile as /(?:)/ ?
Done -