js> eval(""+RegExp("[^/]+$")) typein:1: SyntaxError: unterminated character class ^: typein:1: /[^/]+$/ typein:1: ^ js> build() built on Jun 21 2004 at 18:03:35 (boffo) also tested: mozilla1.4.1 (viper), mozilla1.2.1 (boffo), 1.7b (lab-xp-tp1) afaik RegExp("[^/]+$") should be approximately equivalent to /[^\/]+$/ The testing example I have is: "somepath/file.ext".match(regexpobject) expected output is: "file.ext". since I'm using the regular expression twice, I figured I'd construct and store the object instead of inlining it. Since I'm not particularly fond of escaping things, I decided to use the RegExp() form. The form works, but xpcshell/js and venkman all showed sketchy string representations. Note that the non RegExp constructor /[^\/]+$/ form does correctly unescape. The nature of the error message for my testcase will vary depending on whether the build you're using is from before or after the regexp landing (e.g. 1.2.1 output): js> eval(""+RegExp("[^/]+$")) typein:1: SyntaxError: unterminated character class [: typein:1: /[^/]+$/ typein:1: ^ js> build() built on Nov 30 2002 at 14:36:14 Things to test: /[^\/]+$/ RegExp("[^\\\/]+$") RegExp("[^\\/]+$") RegExp("[^\/]+$") RegExp("[^/]+$") Personally, I expect all of the preceding to behave the same (both as regular expressions and as things convertable to strings).
Summary: Unescape of character class [^/] is broken → Unescape of character class [^/] constructed with RegExp constructor is broken
Ah, sweet RegExp. I think we're up to about 800 cuts now.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Priority: -- → P3
Hardware: PC → All
Target Milestone: --- → mozilla1.8alpha2
js> RegExp("/") ///
It's true that the toString/toSource representation isn't lot of use as eval-fodder, but IE(6) does the same thing, so I'm not sure we can change toString. Maybe toSource? Teetering towards WONTFIX. Note that the RegExp(RegExp("/").source) works, though.
Not js1.5 -- breaking compat isn't a release requirement, usually. (Wonder what 4.x does.)
Status: NEW → ASSIGNED
Mozilla/4.8 [en] (Windows NT 5.0; U): (RegExp("/")).toSource() => /// Mozilla/4.8 [en] (Windows NT 5.0; U): (RegExp("/")).toString() => ///
Created attachment 152453 [details] [diff] [review] proposed fix Per ECMA-262 Edition 3, 184.108.40.206. /be
See the NOTE in 220.127.116.11 -- it allows "///" to be the result of RegExp("/").toString(). The spec also allows what the patch here does, which seems better enough to be worth doing. /be
Comment on attachment 152453 [details] [diff] [review] proposed fix Seems kinda a shame that we have this match_or_replace thing that can do _one_ text replacement for us, but not a global string replacement. Still...
Attachment #152453 - Flags: review?(shaver) → review+
match_or_replace can work globally. However, calling it from the engine to replace / with \/ is tricky: you need a regexp with a negative lookbehind asserting that \ does not precede / (either / is at the front of the string, or there's no \ char before the /). Seems better to open-code it (and not to worry about avoiding reallocs). /be
Created attachment 152477 [details] [diff] [review] proposed fix, v2 Oops -- first patch sucked, forgot to scale length by sizeof(jschar). /be
Comment on attachment 152477 [details] [diff] [review] proposed fix, v2 Even better. r=shaver
Attachment #152477 - Flags: review?(shaver) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
1.4.3? Whatever. Timeless, you can do the work. I actually would rather *not* make this change in any branch, since we were ECMA-conformant, and backward compatible, till this went in for 1.8a2. What's your requirement, exactly? /be
Would like to sync up aviary 1.0 with current trunk in this regard. /be
i'm not likely to use 1.4.3, but i am likely to use 1.7.x. I'm fine with not doing 1.4.3.
I am against this landing on the 1.4 branch.
Comment on attachment 152477 [details] [diff] [review] proposed fix, v2 Per drivers' edict that 1.7x and aviary1.0 back-end modules match, I'm plusing for 1.7.3. /be
Attachment #152477 - Flags: approval1.7.3? → approval1.7.3+
Fixed in 1.7 branch. /be
Keywords: fixed-aviary1.0, fixed1.7, js1.5
js1_5/Regress/regress-248444.js checked in.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.