498636 - Simplify escaping code in nsXHTMLContentSerializer.cpp

Status: NEW

(Core :: DOM: Serializers, defect, P5)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

from bug 498433 comment 3 and bug 498433 comment 7:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsXHTMLContentSerializer.cpp#227
227   // Loop and escape parts by avoiding escaping reserved characters (and '%', '#' ).
228   while ((end = uri.FindCharInSet("%#;/?:@&=+$,", start)) != -1) {
229     part = Substring(aURI, start, (end-start));
230     if (textToSubURI && !IsASCII(part)) {
231       rv = textToSubURI->ConvertAndEscape(mCharset.get(), part.get(), getter_Copies(escapedURI));


  -------  Bug 498433 Comment #3 From  Boris Zbarsky (:bz) (todo: 175+ items)   2009-06-15 15:59:56 PDT -------

(From update of attachment 383323 [details] [diff] [review])
I suppose this is ok, but it's not clear why we're doing escaping using
nsITextToSubURI here in the first place... can you file a bug on making this
sane (e.g. just using NS_Escape with the right flags)?

  -------  Bug 498433 Comment #4 From  Christian :Biesinger   2009-06-16 04:19:06 PDT -------

nsITextToSubURI also does charset conversion before escaping, right? Are you
saying that this code should just always use UTF-8?

  -------  Bug 498433 Comment #7 From  Boris Zbarsky (:bz) (todo: 175+ items)   2009-06-16 08:06:35 PDT -------

> Are you saying that this code should just always use UTF-8?

Possibly, yes.  At least something to seriously consider.

--

That especially makes sense now that we always use UTF-8 for the path of the URI (though not for the query).

Comment 1

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=3 years untouched DOM/Storage bug's priority.

If you have reason to believe this is wrong, please write a comment and ni :jstutte.