Last Comment Bug 718518 - Remove all usages of non-standard flags argument to String.prototype.replace
: Remove all usages of non-standard flags argument to String.prototype.replace
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Till Schneidereit [:till]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-16 14:15 PST by Till Schneidereit [:till]
Modified: 2012-01-19 02:51 PST (History)
10 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Change usages of String.prototype.replace to remove third argument (7.30 KB, patch)
2012-01-16 14:15 PST, Till Schneidereit [:till]
dcamp: review+
jgriffin: review+
dao+bmo: review+
ted: review+
dtownsend: review+
Details | Diff | Review

Description Till Schneidereit [:till] 2012-01-16 14:15:27 PST
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:
"search\s*\(\s*[^/]|match\s*\(\s*[^/]|replace\s*\(\s*[^/]"

This might make bug 481738 more feasible.
Comment 1 Till Schneidereit [:till] 2012-01-16 14:22:06 PST
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] 2012-01-17 08:55:31 PST
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 [:bz] (Out June 25-July 6) 2012-01-18 13:13:03 PST
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] 2012-01-18 13:30:56 PST
Thanks for the checkin and the feedback on the patch format. Will improve that for my next patch.
Comment 5 Josh Matthews [:jdm] 2012-01-18 13:59:17 PST
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.

Note You need to log in before you can comment on or make changes to this bug.