Closed
Bug 437101
Opened 17 years ago
Closed 13 years ago
Remove RegExp.$N for Mozilla 2
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 560440
mozilla2.0
People
(Reporter: jsp, Unassigned)
Details
Attachments
(2 files)
JavaScript's global RegExp object has nine special properties, $1 ... $9, that contain the first nine matches of the last regular expression evaluation. These values are cleared unpredictably. The attached test case demonstrates the only reliable method I have found for eliciting this behavior: make a call to alert(). I am hoping that the test case will lead to discovery and correction of the problem that leads to these values being cleared (much less predictably) in other cases.
Fooey. I tried the test case on another machine and it doesn't manifest the problem. So much for reproducibility. Time to start knocking down extensions, I guess.
I thought I'd filed a comment yesterday to the effect that disabling the IE Tab extension seemed to make the problem go away. However, now that I've installed RC 2, IE Tab no longer seems to be guilty. Today it's Firebug. While Firebug is a more plausible suspect, it's a bit worrisome that a different extension is now implicated. It's like the extensions aren't really guilty, just exposing a bug in the browser. I'll keep plugging away.
I uninstalled RC 2 and installed RC 1. Firebug now looks like the culprit with RC 1 as well. I'm not sure what was going on yesterday. Most likely user error. Bottom line: I think the problem is with Firebug (and only Firebug). Marking INVALID, which I think is the appropriate resolution.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → INVALID
For crying out loud. Now it looks like IE Tab is the culprit again. Disabling Firebug has no effect, but the problem goes away if I disable IE Tab and reappears if I enable it again. I'm back to thinking that extensions may trigger the problem but are not the actual cause. Reopening.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Comment 5•17 years ago
|
||
As far as I understand it, Regexp is a global so its state is not predictable if any function calls intervene between the exec() call and use of the $ values. In my opinion this is a design flaw in Regexp, not something any extension can fix.
Good point. My test case only fails after a call to alert(), which supports your point. However, the test case works in the absence of any extensions, as well as with Firefox 2, Opera 9, and Safari 3.1. Furthermore, the code that cause me to file this bug does not make any function calls between evaluating the regex and using the globals. Here it is: if (elmImage && elmImage.src && reDynImgComps.test(elmImage.src)) { elmImage.src = RegExp.$1 + RegExp.$2 + "M" + RegExp.$4; if (!elmImage.strIndStart) elmImage.strIndStart = RegExp.$3; } The assignment to elmImage.src always works as expected, but RegExp.$3 intermittently gets cleared, so the assignment to elmImage.strIndStart does not work as expected. The following trivial revision seems to work reliably, which is bizarre: if (elmImage && elmImage.src && reDynImgComps.test(elmImage.src)) { if (!elmImage.strIndStart) elmImage.strIndStart = RegExp.$3; elmImage.src = RegExp.$1 + RegExp.$2 + "M" + RegExp.$4; }
Comment 7•17 years ago
|
||
You might try something like var m = reDynImgComps.exec(elmImage.src) if (m) { elmImage.strIndStart = m[3]; ... I didn't think regexp.test() was good for anything beyond true/false.
Yeah, I've considered that, but I don't want to modify code that has worked in diverse environments for almost five years without understanding why. I don't think there's anything logically incorrect about it as it stands, which is why I filed the bug.
The IETAB implements a nsIContentPolicy object to filter the URIs, and do some Regexp match in nsIContentPolicy.shouldLoad(). In your TestCase, the alert() function will cause Firefox triggers the nsIContentPolicy.shouldLoad() in background, and the RegExp is a global object. That is why the RegExp's $1..$9 value is unpredictably.
Reporter | ||
Comment 10•17 years ago
|
||
Could IE Tab's nsIContentPolicy.shouldLoad() also get called in the absence of an alert() call, as described in comment #6? The alert() call makes this problem appear reliably, but I care about that case only to the extent that it helps understand the problem. The real code makes no alert() call, but manifests the problem unpredictably. If, in fact, some other script can get run in the middle of the first code snippet in comment #6, then John is right: the RegExp global is useless and I should use exec().
Comment 11•17 years ago
|
||
I've created a sample extension. You can install the attached bug437101.xpi. It simply implements a nsIContentPolicy object. In your test case page, the RegExp.$1 will show "Hello bug#437101" and RegExp.$2 will show who call the nsIContentPolicy.shouldLoad(). [Code] shouldLoad: function(contentType, contentLocation, ...) { var text = "Hello bug#437101, contentLocation: " + contentLocation.spec; var m = /^(.*,)(.*)$/.test(text); return (Components.interfaces.nsIContentPolicy.ACCEPT); }, The global RegExp.$1 and $2 will be replaced by above RegExp.
Reporter | ||
Comment 12•17 years ago
|
||
That's very helpful! If I create a new profile and install the sample extension, my original script fails. I think that confirms yuoo2k's analysis. So, in Firefox 3, the global RegExp.$N values are subject to change at any time and should therefore be avoided. This is a regression. I'd argue that it violates the RegExp description in http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Global_Objects:RegExp, and I can't find any indication that support for RegExp.$N was dropped in JavaScript 1.6, 1.7, or 1.8.
Assignee: nobody → general
Status: REOPENED → NEW
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Comment 13•16 years ago
|
||
We wish to drop support for these poor-man's perl4-ish dynamically scoped variables. I have no idea what change caused you pain. It was not intended, but by their nature they can be clobbered by nesting RegExp usage on the same JS context (interpreter instance, one per window in Firefox and other Gecko DOM embeddings). I guess Firebug does use regexps on the same context. Please use proper match array results instead. If you can do that, then I would like to morph this bug into "remove RegExp.$1, etc." and target it at Mozilla 2. Thanks, /be
Reporter | ||
Comment 14•16 years ago
|
||
I'll do that. Unfortunately, I just let a product get released using RegExp.$N in the hope that the old behavior would be restored in a subsequent release of Firefox. I still think that's a good idea until Mozilla 2, since this is a documented idiom supported by every browser I've tried, but I can see how it may not be possible.
Comment 15•16 years ago
|
||
The behaviour of the JS engine has not changed; there's nothing to restore. Extensions that use RegExp can affect things, clearly, but that's also nothing new as far as I can tell. We could maybe add some clutter (and overhead, not needed in 99% of cases) to save and restore RegExps, but they're fragile by design and specification, so the right thing is to code conservatively and capture RegExp.$N before calling out. Debuggers might still foil you, but debuggers can foil you anyway, because they can cause arbitrary effects.
Reporter | ||
Comment 16•16 years ago
|
||
Thanks to you all for persisting. I get it now. (It helps that I've just reproduced the problem with Firefox 2 and Firebug. I can't reproduce it with the version of IE Tab that works with Fx 2, so I wonder if some change in IE Tab 1.5 causes the problem to manifest more frequently.) Anyway, I've obviously been living on borrowed time, regardless of what happened to bring the problem to my attention. Morphing the bug per Brendan makes sense. Some mention in the Core JavaScript Reference of the dangers (and intended deprecation) of these properties might help the innocent be suitably wary.
Updated•16 years ago
|
Keywords: dev-doc-needed
OS: Windows XP → All
Hardware: PC → All
Summary: Global RegExp object's $n properties cleared unpredictably → Remove RegExp.$N for Mozilla 2
Updated•16 years ago
|
Target Milestone: --- → mozilla2.0
Comment 17•16 years ago
|
||
Thanks for this Topic, I spend a couple of days trying to fix some errors related to this "bug". Many people reported an Error in an Application, but I could'nt see any problem, no matter if Firebug or IETab is disabled or not. In my case the reason was the Adblock-AddOn. Of course in the begin we checked all AddOns, including Adblock, by disabling them. But it seems, Adblock still is running, if it's disabled via statusbar->Adblock->disable Adblock. If you whitelist the page containing the Problem([Adblock->whitelist->this whole page]) or deactivate Adblock via [Menubar->Options->Addons->Adblock->Deactivate], the Problem is gone.
Comment 18•16 years ago
|
||
(In reply to comment #13) > We wish to drop support for these poor-man's perl4-ish dynamically scoped > variables. Won't this risk unnecessarily breaking quite some legacy code, considering that all major browsers support these properties? Googling for "RegExp.$1" yields quite some results, as does <http://www.koders.com/?s=RegExp.%241&la=JavaScript>. Wouldn't emitting strict warnings when these global properties are accessed make more sense (at least as a first step, anyway)?
Comment 19•13 years ago
|
||
Using codesearch and also including all the other $0 - $9 yields 17k. http://google.com/codesearch#search/&q=RegExp\.\$[0-9]%20lang:javascript&type=cs Also from looking at the result jquery 1.4 used RegExp.$1, which i guess is still around today.
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 17 years ago → 13 years ago
Resolution: --- → DUPLICATE
Updated•13 years ago
|
Keywords: dev-doc-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•