Bug 8395 - REGRESSION: RegEx seems broken for hex escaped non breaking space
Summary: REGRESSION: RegEx seems broken for hex escaped non breaking space
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Darin Adler
URL:
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2006-04-14 13:57 PDT by Bakafish
Modified: 2006-07-15 08:24 PDT (History)
5 users (show)

See Also:


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+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bakafish 2006-04-14 13:57:37 PDT
The RegEx engine is not behaving correctly for special characters (Octal escaped   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>
Comment 1 Alexey Proskuryakov 2006-06-26 06:52:28 PDT
Created attachment 9047 [details]
test case

Same test as an attachment.
Comment 2 Alice Liu 2006-07-05 08:40:32 PDT
<rdar://problem/4613467>
Comment 3 Darin Adler 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.
Comment 4 Darin Adler 2006-07-14 08:57:42 PDT
Created attachment 9448 [details]
patch with detailed change log and a layout test
Comment 5 David Kilzer (:ddkilzer) 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?
Comment 6 David Kilzer (:ddkilzer) 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".
Comment 7 Geoffrey Garen 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.
Comment 8 Geoffrey Garen 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
Comment 9 Geoffrey Garen 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?
Comment 10 David Kilzer (:ddkilzer) 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.
Comment 11 David Kilzer (:ddkilzer) 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.
Comment 12 Geoffrey Garen 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!
Comment 13 David Kilzer (:ddkilzer) 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.

Comment 14 Darin Adler 2006-07-15 08:24:15 PDT
Committed revision 15455.