Bug 8395

Summary: REGRESSION: RegEx seems broken for hex escaped non breaking space
Product: WebKit Reporter: Bakafish <jason>
Component: JavaScriptCoreAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, ddkilzer, ggaren, ian
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
test case
none
patch with detailed change log and a layout test ggaren: review+

Bakafish
Reported 2006-04-14 13:57:37 PDT
The RegEx engine is not behaving correctly for special characters (Octal escaped &nbsp; anyway.) This shouldn't match \s according to the info I have read, which makes sense, but this script works fine in the current Release version. <html><head> <script type="text/javascript" language="javascript1.2"> <!-- var text = "foo"; var nbsp = "\xa0"; alert("Orig: (" + text + ")"); text = text + nbsp + nbsp + nbsp; alert("After: (" + text + ")"); var re = new RegExp(/\xa0*$/); text = text.replace(re,''); alert("Stripped: (" + text + ")"); //--> </script> </head> <body></body> </html>
Attachments
test case (376 bytes, text/html)
2006-06-26 06:52 PDT, Alexey Proskuryakov 		no flags
Details
patch with detailed change log and a layout test (7.65 KB, patch)
2006-07-14 08:57 PDT, Darin Adler 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2006-06-26 06:52:28 PDT
Created attachment 9047 [details] test case Same test as an attachment.
Alice Liu
Comment 2 2006-07-05 08:40:32 PDT
<rdar://problem/4613467>
Darin Adler
Comment 3 2006-07-14 07:15:29 PDT
Doing more tests I find that \xa0 works fine by itself in a regular expression. It's only failing when it comes right before a *. I suspect the bug is with any character > 127 before a * in a regular expression.
Darin Adler
Comment 4 2006-07-14 08:57:42 PDT
Created attachment 9448 [details] patch with detailed change log and a layout test
David Kilzer (:ddkilzer)
Comment 5 2006-07-14 09:19:26 PDT
(In reply to comment #4) > Created an attachment (id=9448) [edit] > patch with detailed change log and a layout test COMMITTERS: Be ware of applying this patch until Bug 9875 lands! (I'm almost done testing it.) Darin, do the PCRE changes need to be submitted "upstream" back to the PCRE project?
David Kilzer (:ddkilzer)
Comment 6 2006-07-14 09:41:51 PDT
(In reply to comment #5) > Darin, do the PCRE changes need to be submitted "upstream" back to the PCRE > project? After reading the JavaScriptCore changelog, I see that JSC is using a modified PCRE-6.1 that supports UTF-16, so these changes don't make sense to be sent "upstream".
Geoffrey Garen
Comment 7 2006-07-14 11:03:02 PDT
It's worth noting that we have dreams of submitting upstream to PCRE, and, initially, at least, they've said they're amenable by email. We just haven't had the time to work it out.
Geoffrey Garen
Comment 8 2006-07-14 11:06:15 PDT
Comment on attachment 9448 [details] patch with detailed change log and a layout test these non-layout bugs are just too easy
Geoffrey Garen
Comment 9 2006-07-14 11:06:42 PDT
> COMMITTERS: Be ware of applying this patch until Bug 9875 lands! (I'm almost > done testing it.) why?
David Kilzer (:ddkilzer)
Comment 10 2006-07-14 11:18:50 PDT
(In reply to comment #9) > > COMMITTERS: Be ware of applying this patch until Bug 9875 lands! (I'm almost > > done testing it.) > > why? Did you read Bug 9875? The current svn-apply script (and it's had this "feature" for a while now) thinks it can apply patches better than patch(1) can, so all of the svn-property information at the "end" of some of the layout test files will be included in the content of the files when the patch is applied.
David Kilzer (:ddkilzer)
Comment 11 2006-07-14 11:19:56 PDT
(In reply to comment #10) > Did you read Bug 9875? The current svn-apply script (and it's had this > "feature" for a while now) thinks it can apply patches better than patch(1) > can, so all of the svn-property information at the "end" of some of the layout > test files will be included in the content of the files when the patch is > applied. So the work-around is to manually fix the files with property changes in them after using svn-apply until the bug is fixed.
Geoffrey Garen
Comment 12 2006-07-14 14:33:34 PDT
> Did you read Bug 9875? Sorry. I guess it was early for me. For some reason, my brain was convinced that Bug 9875 was the ChangeLog fix to svn-apply!
David Kilzer (:ddkilzer)
Comment 13 2006-07-15 03:55:15 PDT
(In reply to comment #4) > Created an attachment (id=9448) [edit] > patch with detailed change log and a layout test This patch may also fix Bug 9894.
Darin Adler
Comment 14 2006-07-15 08:24:15 PDT
Committed revision 15455.
Note You need to log in before you can comment on or make changes to this bug.