This is untested, but should be trivial enough.

This is untested, but should be trivial enough.

This is untested, but should be trivial enough.

Comment on attachment 355107 [details] [diff] [review] (Bv1) /suite/ part [Checkin: See comment 8] > // Copied from the Links Panel v2.3, http://segment7.net/mozilla/links/links.html > // strip leading and trailing whitespace, and replace multiple consecutive whitespace characters with a single space > function stripWS(text) > { >- var middleRE = /\s+/g; >- var endRE = /(^\s+)|(\s+$)/g; >- >- text = text.replace(middleRE, " "); >- return text.replace(endRE, ""); >+ return text.trim().replace(/\s+/g, " "); > } Nit: the first comment line no longer makes sense. >+ var tmp = node.nodeValue.replace(/(\n|\r|\t)+/g, " ").trim(); Nit: might as well put the trim first >- var url = site.value.replace(/^\s*([-\w]*:\/+)?/, "http://"); >+ var url = site.value.trimLeft().replace(/^([-\w]*:\/+)?/, "http://"); >- var site = textfield.value.replace(/^\s*([-\w]*:\/+)?/, ""); >+ var site = textfield.value.trimLeft().replace(/^[-\w]*:\/+/, ""); I don't think these changes improve the code. Please remove them.

Comment on attachment 355106 [details] [diff] [review] (Av1) Port TB bug 454125 [Checkin: See comment 7] >- var type = aData.type.toLowerCase().replace(/^\s+|\s*(?:;.*)?$/g, ""); >+ var type = aData.type.toLowerCase() >+ .trimLeft().replace(/(?:;.*)?$/g, "").trimRight(); I don't think this change is right (or worthwhile). Please remove it.

Comment on attachment 355108 [details] [diff] [review] (Cv1) /mailnews/ part [Checkin: Comment 9] > function trim(string) > { >- return string.replace(/(^\s+)|(\s+$)/g, ""); >+ return string.trim(); > } We should really fix the callers here.

Comment on attachment 355106 [details] [diff] [review] (Av1) Port TB bug 454125 [Checkin: See comment 7] http://hg.mozilla.org/comm-central/rev/4d00c64e4a28 with comment 5 suggestion(s).

Comment on attachment 355107 [details] [diff] [review] (Bv1) /suite/ part [Checkin: See comment 8] http://hg.mozilla.org/comm-central/rev/4f78e0be4a78 with comment 4 suggestion(s). (In reply to comment #4) > >+ var tmp = node.nodeValue.replace(/(\n|\r|\t)+/g, " ").trim(); > Nit: might as well put the trim first I did not: It would change the behavior from the current code and I don't want to do that in this patch. Besides, I think it's needed like this, in case |replace()| "adds" leading/trailing spaces. Yet, I can do it in a followup patch if you do want this change.

Comment on attachment 355108 [details] [diff] [review] (Cv1) /mailnews/ part [Checkin: Comment 9] http://hg.mozilla.org/comm-central/rev/bb65d1541172 (In reply to comment #6) > > function trim(string) > We should really fix the callers here. I noticed it too, but I preferred to leave this (separate) for bug 220348 (followup)...

I guess you don't care about wallet anymore? http://mxr.mozilla.org/comm-central/source/mozilla/extensions/wallet/signonviewer/resources/content/SignonViewer.js#735

> I guess you don't care about wallet anymore? Since we (and thunderbird) are moving to satchel real soon now, there won't be any consumers left, so nobody cares about wallet any more.