Closed Bug 110028 Opened 23 years ago Closed 21 years ago

IDN rendering in URL bar

Categories

(Core :: Networking, defect)

Product:
Core
Component:
Networking
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.3beta

People

(Reporter: wil, Assigned: wil)

References

(
URL
)

Details

(Keywords: intl)

Attachments

(1 file, 3 obsolete files)

new nsIIDNService.idl with changes suggested by Darin
3.03 KB, text/plain
nhottanscp
: review+
darin.moz
: superreview+
Details 
IDN urls are displayed as ACE format now. It should be rendered as UTF-8.
Ideally the user shouldn't see ACE at all.

The problem is several fold.
Firstly, nsHttpChannel replaces the original UTF-8 hostname in nsIURI.Secondly,
even if the nsIURI hostname was in UTF-8, the URL representation
would be hex-encoded.
Does the current DLL support decoding ACE? Can we add the decoding method to
nsIIDNService?
yes we can. i'll try to do that. what do you reckon we should modify to have it
rendered as UTF-8? Is the text widget for URL bar in UTF-8?
I just thought that the interface which encodes to ACE probably knows how to
decode it too. I think the widget takes UCS2, we already have UTF-8 to UCS2
conversion.

The other issue is where to hook up the decoder, in netwerk as soon as we get
the ACE encoded URL or somewhere else, e.g. right before we put the URL string
to the widget.

Yes the library can decode ACE, so how about using the name 
nsIIDNService::IDNToUTF8HostName()?
Yes, I think that's fine.



*** Bug 120744 has been marked as a duplicate of this bug. ***
Blocks: 124042
Attached patch Adding IDNToUTF8HostName (obsolete) — — Splinter Review 
Is this the right time? Or should we wait for bug 84186? Here it is anyway.
william: you should probably add [noscript] to these methods until bug 84186 is
resolved.  otherwise, sr=darin.

btw: wouldn't ConvertACEtoUTF8 and ConvertUTF8toACE be more appropriate function
names?  IDN isn't an encoding, right?  but UTF8 is.  ACE is an encoding (sort of).
no need for these methods to include the noun 'hostname' IMO, since these
methods are on nsIIDNService, which implies that it is dealing with hostnames.

also, a lowercase 'to' is probably better.. consistent with NS_ConvertUCS2toUTF8
and friends.
Darin: your suggested names are more apt. I don't mind changing them, Naoki?
If this looks okay, i'll submit a patch to update nsHttpChannel and
nsDnsService.
Attachment #68263 - Attachment is obsolete: true
Comment on attachment 69164 [details]
new nsIIDNService.idl with changes suggested by Darin

looks good to me sr=darin
Attachment #69164 - Flags: superreview+
Comment on attachment 69164 [details]
new nsIIDNService.idl with changes suggested by Darin

r=nhotta
Attachment #69164 - Flags: review+
This includes the nsIIDNService.idl diffs.
Comment on attachment 70067 [details] [diff] [review]
search and replace occurances of old nsIIDNService method name

sr=darin

once this lands and my nsIURI changes land, i can make nsIURI automatically
convert from ACE to UTF8!!  this'll make redirected URLs w/ IDN human readable
:-)
Attachment #70067 - Flags: superreview+
Keywords: intl
QA Contact: benc → teruko
Can this make it into 0.9.9?
yes it certainly can... just get an r= (nhotta?) and then send mail to
drivers@mozilla.org requesting permission to land this.  you'll need to act
quickly if you want to get this in.  this is a low-risk API change, so i suspect
they'll be happy to accept.
Naoki, could you please r= attachment 70067 [details] [diff] [review]?
Thanks!
Comment on attachment 70067 [details] [diff] [review]
search and replace occurances of old nsIIDNService method name

r=nhotta
Attachment #70067 - Flags: review+
we should try to land this when the trunk opens for mozilla 1.0

nhotta: can you take care of landing this?
Keywords: mozilla1.0
Target Milestone: --- → mozilla1.0
okay
No longer blocks: 124042
Blocks: 124042
Comment on attachment 70067 [details] [diff] [review]
search and replace occurances of old nsIIDNService method name

a=shaver for the 1.0 trunk.
Attachment #70067 - Flags: approval+
No longer blocks: 124042
checked in to the trunk
As instructed by Naoki, this should make it easier for users to try out IDN
with
Mozilla. This does not change the behavior if XPIDN was not installed.
william: i think that patch belongs in another bug, but if nhotta asked to have
it here, then no problem... sr=darin.
Comment on attachment 73604 [details] [diff] [review]
Turn on network.enableIDN by default.

sr=darin
Attachment #73604 - Flags: superreview+
Comment on attachment 73604 [details] [diff] [review]
Turn on network.enableIDN by default.

r=nhotta
Attachment #73604 - Flags: review+
Comment on attachment 73604 [details] [diff] [review]
Turn on network.enableIDN by default.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #73604 - Flags: approval+
checked in to the trunk
Comment on attachment 70067 [details] [diff] [review]
search and replace occurances of old nsIIDNService method name

obsoleting checked in patches.
Attachment #70067 - Attachment is obsolete: true
Comment on attachment 73604 [details] [diff] [review]
Turn on network.enableIDN by default.

obsoleting checked in patches so they don't look like approved changes that
haven't landed yet.
Attachment #73604 - Attachment is obsolete: true
Shouldn't this bug be closed ?
yes, this is fixed now
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.0 → mozilla1.3beta
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: