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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

Attachment: 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:
"search\s*\(\s*[^/]|match\s*\(\s*[^/]|replace\s*\(\s*[^/]"

This might make bug 481738 more feasible.

Comment 1

[Till Schneidereit [:till]]

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

[Luke Wagner [:luke]]

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 hg.mozilla.org/mozilla-central, 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.)

Comment 3

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f8b66a93333

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!

Comment 4

[Till Schneidereit [:till]]

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

Comment 5

[Josh Matthews [:jdm]]

http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed is a great reference for how to set all that up for future patches.

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/0f8b66a93333

you can also reference mdn updated entry
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F