Currently we (re-)escape the whole string when it contains a forbidden char: http://mxr.mozilla.org/mozilla-central/source/intl/uconv/nsTextToSubURI.cpp#266 It would be better if we only (re-)escape the forbidden chars and leave the rest alone. Also, FindCharInSet(unsafeChars) is rather slow but since we know 'unsafeChars' is very likely sorted we can optimize this by doing a binary search.

Comment on attachment 8527771 [details] [diff] [review] part 1 - Don't escape all non-ASCII characters when unsafe characters are found, just escape the unsafe characters. Review of attachment 8527771 [details] [diff] [review]: ----------------------------------------------------------------- LGTM ::: xpcom/io/nsEscape.h @@ +170,5 @@ > + * Percent-escapes all characters in aStr that occurs in aForbidden. > + * @param aStr the input URL string > + * @param aForbidden the characters that should be escaped if found in aStr > + * @note that aForbidden MUST be sorted (low to high) > + * @param aResult the result if some some characters were escaped Nit: remove the extra "some"

(In reply to Simon Montagu :smontagu from comment #3) > Nit: remove the extra "some" Fixed. https://hg.mozilla.org/integration/mozilla-inbound/rev/be23ef5730dc https://hg.mozilla.org/integration/mozilla-inbound/rev/c2301fd8921b

https://hg.mozilla.org/mozilla-central/rev/be23ef5730dc https://hg.mozilla.org/mozilla-central/rev/c2301fd8921b

I landed a trivial fixup for mingw (nsString using char16ptr_t causing ambiguous call): https://hg.mozilla.org/integration/mozilla-inbound/rev/c4a4eceb85f2