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

RESOLVED FIXED in mozilla12



JavaScript Engine
6 years ago
6 years ago


(Reporter: till, Assigned: till)


Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)



(1 attachment)



6 years ago
Created attachment 589006 [details] [diff] [review]
Change usages of String.prototype.replace to remove third argument

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)

Comment 1

6 years ago
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 2

6 years ago
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)


6 years ago
Attachment #589006 - Flags: review?(dao) → review+


6 years ago
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+
Keywords: checkin-needed


6 years ago
Assignee: general → tschneidereit+bmo


6 years ago
Ever confirmed: true

Comment 3

6 years ago

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

Comment 4

6 years ago
Thanks for the checkin and the feedback on the patch format. Will improve that for my next patch.

Comment 5

6 years ago is a great reference for how to set all that up for future patches.

you can also reference mdn updated entry
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.