1101625 - make UnEscapeURIForUI escape blacklisted chars only

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Mats Palmgren (inactive)]

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 1

[Mats Palmgren (inactive)]

Attachment: part 1 - Don't escape all non-ASCII characters when unsafe characters are found, just escape the unsafe characters.

Comment 2

[Mats Palmgren (inactive)]

Attachment: part 2 - Remove the aIRI param since it's always true. And some minor code cleanup.

https://tbpl.mozilla.org/?tree=Try&rev=87e9ae71fd22

Comment 3

[Simon Montagu :smontagu]

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"

Comment 4

[Mats Palmgren (inactive)]

(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

Comment 5

[Phil Ringnalda (:philor)]

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

Comment 6

[Jacek Caban]

I landed a trivial fixup for mingw (nsString using char16ptr_t causing ambiguous call):

https://hg.mozilla.org/integration/mozilla-inbound/rev/c4a4eceb85f2

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c4a4eceb85f2