99478 - cannot round trip convert ' ' between nsTextToSubURI::ConvertAndEscape and UnEscapedAndConvert

Status: NEW

(Core :: Internationalization, defect)

Description

[Bradley Baetz (:bbaetz)]

The call to nsEscape (see url above) uses url_XPAlphas, which changes spaces to
pluses. This means that taking a string, calling ConvertAndEscape, and then
UnEscapeAndConvert on the result won't give you back the same string you started
with.

The one line diff is to change url_XPAlphas to url_XAlphas (I don't have a
machine I can generate a diff on ATM) See
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsEscape.cpp#31 for a
description of the bits.

Comment 1

[Bradley Baetz (:bbaetz)]

Well, its a description of a patch...

Comment 2

[Roy Yokoyama]

Trying ftang

Comment 3

[Frank Tang]

can you tell me which usage of this cause problem ?

Comment 4

[Bradley Baetz (:bbaetz)]

There aren't any yet. I wanted to use this routine as part of bug 78148, but
then directories with spaces in the name stopped working. I think that I have a
workarround, but I won't know until I get i18n feedback from that bug. This
routine should be fixed anyway.

Comment 5

[Frank Tang]

Change summary from 
"nsTextToSubURI::ConvertAndEscape changes ' ' to '+'"
to 
"cannot round trip convert ' ' between nsTextToSubURI::ConvertAndEscape and 
UnEscapedAndConvert"

In UnEscapedAndConvert, we need to chagne '+' back to ' ', currently, we didn't 
do it. 

Comment 6

[Bradley Baetz (:bbaetz)]

Just to follow on from the discussion I had with ftang on IRC, existing callers
which do this (eg
http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/src/nsInternetSearchService.cpp#2635)
will probably have to be modified if the fixup is no londer required.

Comment 7

[Frank Tang]

nhotta, can you take a look at this one ?

Comment 8

[nhottanscp]

The bug has keywords 'patch' and 'review' but I cannot see the patch.
Is this proposing to change nsTextToSubURI or nsEscape?

Comment 9

[Bradley Baetz (:bbaetz)]

Yeah, scrap those. My intended fix (Described in the opening comment) is wrong -
also see my 2001-09-28 14:14 comment.

Comment 10

[nhottanscp]

The round trip problem if exists that is an issue of nsEscape not nsTextToSubURI
(which is just calling nsEscape).
If that's the case then this is not i18n bug (may go to xpcom).

Comment 11

[Bradley Baetz (:bbaetz)]

No. The problem (as explained to me by ftang) is this:

consider
http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/src/nsInternetSearchService.cpp#2635
What we need is a url, so we ask nsEscape to convert ' ' to '+' (as part of the
url). Then we want to go backwards. The code that is in the url above is needed
to do so correctly. nsITextToSubURI should do this for its clients.

(I'm changing the severity, since this can be worked arround, just as the
internetsearchservice does. It just seems silly to have to do this. There are
only a couple of users of this function, and they all do so)

Comment 12

[nhottanscp]

The code in nsInternetSearchService.cpp avoids '+' to be escaped as '%2B'.

example:
http://www.google.com/search?q=john+lennon

http://www.google.com/search?q=john%2Blennon

I agree that it is not convenient for the client code to work around it but the
escaping itself is done in nsEscape not in i18n code.
There is a new function nsStdEscape which seems to be avoiding over escaping.
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsEscape.cpp#326
So, nsITextToSubURI could add a function like this.

Comment 13

[Frank Tang]

both naoki and I are off mozilla for about 20 monthes. If these bugs are still
here, the real status is 'wont fix'. If you want to reopen it, please find a
better owner who really looking at the bug database now. 

Comment 14

[Travis Chase]

Mass Reassign Please excuse the spam

Comment 15

[Travis Chase]

Mass Re-opening Bugs Frank Tang Closed on Wensday March 02 for no reason, all
the spam is his fault feel free to tar and feather him

Comment 16

[Travis Chase]

Reassigning Franks old bugs to Jungshik Shin for triage - Sorry for spam

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.