Closed Bug 718518 Opened 10 years ago Closed 10 years ago

Remove all usages of non-standard flags argument to String.prototype.replace


(Core :: JavaScript Engine, defect)

Not set





(Reporter: till, Assigned: till)



(1 file)

As part of my analysis in bug 580480, I looked for usages of the optional third argument to String.prototype.{search,match,replace}.

Attached is a patch that removes all usages within mozilla-central found with the following grep command and then pruned down to remove false positives:

This might make bug 481738 more feasible.
Attachment #589006 - Flags: review?(luke)
Oh, and Luke, sorry if asking you for a review wasn't right. The nature of this cleanup makes it a bit hard to find a good reviewer (or even the right component).
Comment on attachment 589006 [details] [diff] [review]
Change usages of String.prototype.replace to remove third argument

In theory the changes are simple enough I could review them, but I have not reviewed frontend JS before so I don't think I should.  Also, it would be good to spread the word about not using non-standard regexp flags.

To find reviewers, you can go to, browse to the file, then click 'annotate' and find either the last person that touched the line or someone who has touched most of the file.  Doing that:

dcamp, could you review CssRuleView.jsm;
jgriffin, could you review tps.jsm;
dao, could you review LightweightThemeConsumer.jsm;
ted, could you review CrashSubmit.jsm; and
dtownsend, could you review AddonRepository.jsm

?  Hopefully I didn't mess that up :)

(The use in XPIProvider.jsm seems to have been removed.)
Attachment #589006 - Flags: review?(ted.mielczarek)
Attachment #589006 - Flags: review?(luke)
Attachment #589006 - Flags: review?(jgriffin)
Attachment #589006 - Flags: review?(dtownsend)
Attachment #589006 - Flags: review?(dcamp)
Attachment #589006 - Flags: review?(dao)
Attachment #589006 - Flags: review?(dao) → review+
Attachment #589006 - Flags: review?(dcamp) → review+
Attachment #589006 - Flags: review?(jgriffin) → review+
Attachment #589006 - Flags: review?(ted.mielczarek) → review+
Attachment #589006 - Flags: review?(dtownsend) → review+
Assignee: general → tschneidereit+bmo
Ever confirmed: true

In the future, a checkin comment and User line would be really nice to have in the changeset.  Wasn't too big a problem for this bug, though.

Thank you for the patch!
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla12
Thanks for the checkin and the feedback on the patch format. Will improve that for my next patch. is a great reference for how to set all that up for future patches.
You need to log in before you can comment on or make changes to this bug.