Last Comment Bug 112979 - open source for nsIIDNService implementation
: open source for nsIIDNService implementation
Status: VERIFIED FIXED
: intl
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.3beta
Assigned To: nhottanscp
: Yuying Long
: Patrick McManus [:mcmanus]
Mentors:
http://lxr.mozilla.org/seamonkey/sour...
Depends on: 8275
Blocks: 92109 162571 188215 188218
  Show dependency treegraph
 
Reported: 2001-11-30 16:37 PST by nhottanscp
Modified: 2003-07-03 11:19 PDT (History)
20 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test code to encode stringprep date to C code. (37.59 KB, patch)
2002-06-20 18:00 PDT, nhottanscp
no flags Details | Diff | Splinter Review
Adding empty IDN implementation to make actual implementation done easily. (12.74 KB, patch)
2002-09-25 13:30 PDT, nhottanscp
shanjian: review+
darin.moz: superreview+
Details | Diff | Splinter Review
Integrate JPNIC nameprep/race, pending license issue. (37.01 KB, patch)
2002-10-11 10:10 PDT, nhottanscp
no flags Details | Diff | Splinter Review
string prep mapping table for the previous patch (317.81 KB, patch)
2002-10-11 10:12 PDT, nhottanscp
no flags Details | Diff | Splinter Review
Added punycode implementation from the draft. (52.69 KB, patch)
2002-10-24 14:19 PDT, nhottanscp
no flags Details | Diff | Splinter Review
Changed "IESG--" as a default prefix, RACE can be specified by a pref. (52.58 KB, patch)
2002-10-24 15:46 PDT, nhottanscp
no flags Details | Diff | Splinter Review
Updated JPNIC license. (49.36 KB, patch)
2002-12-09 17:28 PST, nhottanscp
no flags Details | Diff | Splinter Review
JPNIC source files (36.05 KB, patch)
2003-01-08 13:39 PST, nhottanscp
no flags Details | Diff | Splinter Review
xpcom glue code (13.31 KB, patch)
2003-01-08 13:41 PST, nhottanscp
ftang: review+
darin.moz: superreview-
Details | Diff | Splinter Review
JPNIC code based on idnkit-1.0pr2 (42.36 KB, patch)
2003-01-08 14:21 PST, nhottanscp
no flags Details | Diff | Splinter Review
JPNIC nameprep data from idnkit-1.0pr2 (117.31 KB, patch)
2003-01-08 14:22 PST, nhottanscp
nhottanscp: review+
darin.moz: superreview+
Details | Diff | Splinter Review
xpcom glue code (address reviewer's comments) (13.59 KB, patch)
2003-01-09 14:05 PST, nhottanscp
darin.moz: superreview+
Details | Diff | Splinter Review
JPNIC code based on idnkit-1.0pr2 (added comment about the mapping table) (42.51 KB, patch)
2003-01-09 14:20 PST, nhottanscp
nhottanscp: review+
darin.moz: superreview+
Details | Diff | Splinter Review
instruction for testing (1.20 KB, text/html)
2003-01-21 10:59 PST, nhottanscp
no flags Details

Description nhottanscp 2001-11-30 16:37:56 PST
nsIIDNService implementation is currently not a part of Mozilla.
Comment 1 nhottanscp 2001-11-30 16:41:03 PST
Adding a link to nsIIDNService.idl in LXR.
We also need a method to convert from ACE to Unicode (see bug 110028).
Also we might want a method to apply NamePrep, so NamePrep and ACE encoding can
be done separately.
Comment 2 nhottanscp 2001-12-01 10:22:09 PST
cc to JPNIC
Comment 3 Katsuhiko Momoi 2001-12-27 09:33:35 PST
Added JDNA's ishisone-sab. Other JPN people are also from JDNA.
Comment 4 nhottanscp 2002-01-29 13:08:52 PST
cc to darin

From e-mail conversation, I heard that the development has started, so set to 0.9.9.
Comment 5 Darin Fisher 2002-01-29 13:19:11 PST
it has come to my attention that the current interface ConvertUTF8toIDN is
technicially incorrect and essentially non-scriptable.  the problem is that
|string| is interpreted by xpconnect as ASCII.  as a result, JS code which
passes a non ASCII string into ConvertUTF8toIDN will have the high bytes chopped
off.

either this interface needs to be modified to take wstring (ie.
ConvertUCS2toIDN) or we need bug 84186 to be solved (which will add a UTF8
string type to XPCOM).
Comment 6 nhottanscp 2002-01-29 13:31:12 PST
We probably need to change it to noscript. I don't think IDN encoding is needed
from scripting code.
Comment 7 nhottanscp 2002-01-29 14:14:30 PST
The patch is not ready for posting but I got some info through e-mail conversation.
Here is a list of issues.

1) The patch is for Unix only (for now).

2) It needs an additional package in order to build (mDNkit).
http://www.nic.ad.jp/jp/research/idn/mdnkit/download/

3) Schedule, can this make 0.9.9 (20-Feb-2002)?
Comment 8 Darin Fisher 2002-01-29 14:57:06 PST
i don't know what mozilla's policy is for adding new external dependencies. 
perhaps the IDN service impl belongs in extensions?

brendan: what do you say?
Comment 9 Yoshiro, YONEYA 2002-01-29 19:07:48 PST
Hi, folks,

I'm very new to here, so please forgive me and let me know if there is any bad
in manner.

As Hotta-san said, now JPNIC (and would succeed to JDNA) is developping open
source nsIIDNService implementation using JPNIC's mDNkit. Our intention is to
enable IDN capability as an usual feature in Mozilla 1.0. To achieve it, we'd
like to hear more advice from you.

Here, I have some questions to proceed our development correctly.

1. External library dependancy
  Our implementation depends on mDNkit's new library (hopefully released as a
part of mDNkit-2.3 in the near future). What is the recommended way to solve
this dependency?

2. Naming rules for Class/Contract ID
  Our implementation currently uses newly generated Class ID and already defined
Contract ID which is "@i-dns.net/IDNService;1" for XPIDN. If there are naming
rules for them, please let us know.

3. Configure option
  We added new configure option to build IDN component with mDNkit as follows:
    --with-mdnkit(=prefix)
      indicates to compile IDN component with mDNkit. prefix is a path name
where mDNkit is installed. default is "/usr/local/".
  If there are nameing rules or policies to add configure option, please let us
know.

3. How to enable IDN feature
  What is the correct way to enable IDN feature in Mozilla? Current XPIDN
requires to access resource:///res/idnconfig.html.

Thanks in advance.
Comment 10 Darin Fisher 2002-01-29 19:18:29 PST
cc'ing seawood and mcafee hoping that they can help with this 'how do i add an
external dependency to mozilla?' question.
Comment 11 hacker formerly known as seawood@netscape.com 2002-01-29 20:35:55 PST
Because win32 & classic mac do not have the concept of installing libraries on a
system to be used for general development, we have historically handled external
dependencies by checking them into the tree.  I really despise that approach &
hope that we can come up with something better.  I don't want to see us becoming
a maintainer of a fork of the external library if it's not necessary.

Anyway, if a component has a new build dependency, it needs to be run by
staff@mozilla.org & the build release team first so that the nightly automation
& tinderboxes can be updated before the new dependency is enabled.

I would prefer to use --with-mdnkit-prefix to tell where mDNkit was installed. 
This module sounds like it would start off life as an optional extension so the
existing --enable-extensions mechanism would work if it were checked in under
mozilla/extensions/ .  Otherwise, we would need an --enable-idn option.
Comment 12 bobj 2002-03-04 09:45:34 PST
FYI, an update on the standards effort:
On 3/4/2002, the IDN WG co-chairs sent an email to the mailing list,
idn@ops.ietf.org, stating that the document authors had summarized the
comments and drafted new versions
 - draft-ietf-idn-idna-07.txt
 - draft-ietf-idn-nameprep-08.txt
 - draft-ietf-idn-punycode-01.txt
 - draft-hoffman-stringprep-01.txt

These documents will be sent for IESG consideration for
Proposed Standard on March 11th 2002.
Comment 13 Yoshiro, YONEYA 2002-05-16 06:36:50 PDT
JPNIC published IDN XPCOM patch for Mozilla-0.9.8 on JPNIC's Web.

1) patch for libnecko
    http://www.nic.ad.jp/ja/idn/mdnkit/download/misc/mozilla-idn-builtin-patch
2) patch for shared library
    http://www.nic.ad.jp/ja/idn/mdnkit/download/misc/mozilla-idn-standalone-patch

Apologize this is out of date and nothing improved since I reported a few
monthes ago.  I hope others contribution to progress the work.

To use this patch, please follow following procedure.

a) download and install mDNkit-2.4
    http://www.nic.ad.jp/ja/idn/mdnkit/download/
b) apply patch which you like to Mozilla-0.9.8-release source
c) create configure with autoconf-2.1.3 (autoconf-2.5 will fail)
d) execute configure with '--with-mdnkit' option
Comment 14 nhottanscp 2002-05-21 12:01:52 PDT
YONEYA san, is it possible to separate components from mdnkit and check in to
Mozilla tree? 
I think Mozilla only needs nameprep and ACE encode/decode.
Comment 15 Yoshiro, YONEYA 2002-05-23 18:24:41 PDT
Hotta-san,

It's hard to extract NAMEPREP and ACE module from current mDNkit.
JPNIC is developping idnkit -- succeeder of mDNkit -- which will be released
within a few months.  It would be a good candidate to incorporate IDN libraries
to Mozilla.  Would you please wait a while?

BTW, JPNIC can't promise being in charge of incorporate work for Mozilla.  Any
other's cooperation is highly recommended :-)
Comment 16 nhottanscp 2002-06-20 17:58:38 PDT
I did some research reading the drafts.

http://www.ietf.org/internet-drafts/draft-ietf-idn-punycode-02.txt
The author put a sample C code in the draft. I put the code to mozilla and
worked fine. But I cannot find a test case, I mean live site to test with.

http://www.ietf.org/internet-drafts/draft-hoffman-stringprep-03.txt
I wrote a test code to estimate the size of mapping table for stringprep.
The result was about 11 kb for the mapping table encoded to a C code.

We also need KC nomalization (compatible decompose and canonical compose). ICU
(International Components for Unicode) has the normilization code. It uses a
binary data about 100 Kb to do the mapping. We need investigation if we can use
this for mozilla.


Comment 17 nhottanscp 2002-06-20 18:00:37 PDT
Created attachment 88567 [details] [diff] [review]
test code to encode stringprep date to C code.
Comment 18 William Tan 2002-06-24 08:53:49 PDT
Hotta-san, you can try http://zq--wgv71a119e.dready.org/ (the nameprepped
version of <kanji-nihongo>.dready.org).
Comment 19 nhottanscp 2002-06-27 15:06:28 PDT
It works if only no ASCII is feeded to the punycode encoder. 
I mean "<kanji-nihongo>" part but input "<kanji-nihongo>.dready.org" produces a
different result, ".dready.org-tj8wq5jvv8nw". 
Is there a spec about how to separate a host name and apply punycode?
Comment 20 William Tan 2002-06-27 21:46:38 PDT
Quoted from http://www.ietf.org/internet-drafts/draft-ietf-idn-nameprep-10.txt

> Nameprep is used to process domain name labels, not domain names. IDNA
> calls nameprep for each label in a domain name, not for the whole domain
> name.

Just split the domain name at U+002E to get the labels.
Comment 21 nhottanscp 2002-06-28 10:12:54 PDT
A prefix "zq--" is used in http://zq--wgv71a119e.dready.org/.

http://www.ietf.org/internet-drafts/draft-ietf-idn-idna-10.txt
section 5 is for ACE prefix which lists several prefixes but does not explain each.
Which prefix do we need to prepend to the encoded string?
Comment 22 Yoshiro, YONEYA 2002-07-04 22:22:01 PDT
JPNIC released IDN XPCOM patch for Mozilla-1.0 libnecko.

  http://www.nic.ad.jp/ja/idn/mdnkit/download/misc/mozilla-1.0-mdnkit-2.4.diff

Please refer to my previous message to apply it and compile.

  http://bugzilla.mozilla.org/show_bug.cgi?id=112979#c13

BTW, "zq--" is implicitly used for Punycode(AMC-ACE-Z)'s testing prefix.
Only "bq--" for RACE and "dq--" for DUDE were registered for testing by IDN WG.

  http://www.i-d-n.net/#ace_registry
Comment 23 William Tan 2002-07-20 20:02:05 PDT
Sorry, U+002E is not the only label separator. Section 3.1 in 
http://www.ietf.org/internet-drafts/draft-ietf-idn-idna-10.txt
says:

> 1) Whenever dots are used as label separators, the following characters
> MUST be recognized as dots: U+002E (full stop), U+3002 (ideographic full
> stop), U+FF0E (fullwidth full stop), U+FF61 (halfwidth ideographic full
> stop).
Comment 24 nhottanscp 2002-07-22 10:26:16 PDT
I see, I was wondering why those are not found in the string prep table.
Comment 25 nhottanscp 2002-09-25 13:30:14 PDT
Created attachment 100615 [details] [diff] [review]
Adding empty IDN implementation to make actual implementation done easily.
Comment 26 nhottanscp 2002-09-25 13:31:27 PDT
darin, could you review the patch?
Comment 27 Darin Fisher 2002-09-25 13:32:58 PDT
Comment on attachment 100615 [details] [diff] [review]
Adding empty IDN implementation to make actual implementation done easily.

sr=darin
Comment 28 Shanjian Li 2002-09-25 13:58:47 PDT
Comment on attachment 100615 [details] [diff] [review]
Adding empty IDN implementation to make actual implementation done easily.

r=shanjian
Comment 29 nhottanscp 2002-09-26 18:22:13 PDT
the patch was checked in to the trunk
the bug is still open since I only checked in the empty implementation
Comment 30 nhottanscp 2002-10-11 10:10:29 PDT
Created attachment 102590 [details] [diff] [review]
Integrate JPNIC nameprep/race, pending license issue.
Comment 31 nhottanscp 2002-10-11 10:12:21 PDT
Created attachment 102591 [details] [diff] [review]
string prep mapping table for the previous patch
Comment 32 bobj 2002-10-15 11:06:29 PDT
Updating list of propose standards:

 - draft-ietf-idn-idna-13.txt: Internationalizing Domain Names In
   Applications (IDNA) (Proposed Standard)

 - draft-ietf-idn-punycode-03.txt: Punycode:A Bootstring encoding
   of Unicode for IDNA (Proposed Standard)  

 - draft-ietf-idn-nameprep-11.txt: Nameprep: A Stringprep Profile
   for Internationalized Domain Names (Proposed Standard)

 - draft-hoffman-stringprep-07.txt: Preparation of
   Internationalized Strings ('stringprep') (Proposed Standard)

For status see http://www.ietf.org/IESG/status.html
Comment 33 nhottanscp 2002-10-24 14:19:54 PDT
Created attachment 104034 [details] [diff] [review]
Added punycode implementation from the draft.

The code was taken from
http://www.ietf.org/internet-drafts/draft-ietf-idn-punycode-03.txt

ACE prefix can be specified by pref "network.IDN_prefix". But I think currently
no prefix is assigned for punycode.
Comment 34 bobj 2002-10-24 15:13:45 PDT
FYI, just received this email that the IDN IDs have been approved
as Proposed Standards by the IESG:

Subject: [idn] Protocol Action: Internationalizing Domain Names In
         Applications (IDNA) to Proposed Standard
Date: Thu, 24 Oct 2002 14:51:18 -0400
From: The IESG <iesg-secretary@ietf.org>

The IESG has approved publication of the following Internet-Drafts
as Proposed Standards:

o Internationalizing Domain Names In Applications (IDNA)
  as a Proposed Standard.  
o Nameprep: A Stringprep Profile for Internationalized Domain Names  
o Punycode: An encoding of Unocode for use with IDNA
Comment 35 nhottanscp 2002-10-24 15:25:07 PDT
According to "Internationalizing Domain Names In Applications (IDNA)",
http://www.ietf.org/internet-drafts/draft-ietf-idn-idna-14.txt
>The ACE prefix for IDNA is "IESG--".

Comment 36 nhottanscp 2002-10-24 15:46:15 PDT
Created attachment 104044 [details] [diff] [review]
Changed "IESG--" as a default prefix, RACE can be specified by a pref.
Comment 37 Teruko Kobayashi 2002-10-24 16:48:13 PDT
Changed QA contact to ylong@netscape.com.
Comment 38 nhottanscp 2002-12-09 17:28:05 PST
Created attachment 108818 [details] [diff] [review]
Updated JPNIC license.
Comment 39 nhottanscp 2003-01-07 17:50:47 PST
RFC 3454: Preparation of Internationalized Strings ("stringprep")
http://www.ietf.org/rfc/rfc3454.txt
Comment 40 nhottanscp 2003-01-08 13:39:56 PST
Created attachment 110978 [details] [diff] [review]
JPNIC source files

separated the last patch into two, JPNIC code and the xpcom glue
Comment 41 nhottanscp 2003-01-08 13:41:07 PST
Created attachment 110979 [details] [diff] [review]
xpcom glue code
Comment 42 nhottanscp 2003-01-08 14:21:25 PST
Created attachment 110981 [details] [diff] [review]
JPNIC code based on idnkit-1.0pr2

idnkit-1.0pr2
http://www.nic.ad.jp/ja/idn/mdnkit/download/
Comment 43 nhottanscp 2003-01-08 14:22:35 PST
Created attachment 110982 [details] [diff] [review]
JPNIC nameprep data from idnkit-1.0pr2

idnkit-1.0pr2
http://www.nic.ad.jp/ja/idn/mdnkit/download/
Comment 44 nhottanscp 2003-01-08 15:48:48 PST
Comment on attachment 110979 [details] [diff] [review]
xpcom glue code

This is xpcom glue for the JPNIC IDN code (which also needs your review later).
Comment 45 nhottanscp 2003-01-08 16:40:41 PST
Yoneya san,
I started to look at the nameprep implementation. There are three mapping tables
and some macros are defined. I can see that it generates an offset from the
input code point and map to one or more code points, but not clear about the detail.
Is it possible to put some comments in nameprep_template.c or nameprepdata.c to
explain about those tables?
Comment 46 Darin Fisher 2003-01-08 20:59:12 PST
Comment on attachment 110979 [details] [diff] [review]
xpcom glue code

nhotta: why do you need to make these changes to the DNS service?  why can't
everything live behind nsIIDNService?  why can't this service be implemented
inside mozilla/intl/?
Comment 47 Darin Fisher 2003-01-08 21:01:30 PST
in fact, nsDnsService should only need to deal with ASCII hostnames.  nsIURI
implmentations (such as nsStandardURL) should provide the mapping from UTF-8
hostnames to ACE hostnames (see nsIURI::asciiHost).
Comment 48 nhottanscp 2003-01-09 09:58:10 PST
The patch implements nsIIDNService. Unicode normalization is implemented in
mozilla/intl (bug 8275), the stringprep and ACE implementation are specific to
IDN so implemented in nsIIDNService. 
Do you mean we should implement stringprep and ACE in nsIURI then nsIIDNService
to call them?
Comment 49 Darin Fisher 2003-01-09 11:35:49 PST
nhotta: i'm sorry for the confusion.  i misread your patch and thought you were
modifying the DNS service directly.  modifying nsIDNService is completely
correct.  sorry about that!! ;-)
Comment 50 Darin Fisher 2003-01-09 11:57:53 PST
Comment on attachment 110979 [details] [diff] [review]
xpcom glue code

>+CSRCS     = race.c  \
>+            nameprep.c \
>+            punycode.c

so these files are going to be added to necko as well?


>Index: dns/src/nsIDNService.cpp

>+  nsCOMPtr<nsIPref> prefs(do_GetService(NS_PREF_CONTRACTID, &rv));
>+  if (NS_SUCCEEDED(rv)) {
>+
>+    // to support test environment
>+    PRBool value;
>+    rv = prefs->GetBoolPref("network.IDN_testbed", &value);
>+    if (NS_SUCCEEDED(rv))
>+      mMltbd = value;
>+
>+    // read prefix from pref
>+    nsXPIDLCString prefix;
>+    rv = prefs->GetCharPref("network.IDN_prefix", getter_Copies(prefix));
>+    if (NS_SUCCEEDED(rv) && 
>+        prefix.Length() == kACEPrefixLen)
>+      strncpy(mACEPrefix, prefix.get(), kACEPrefixLen);
>+  }

mMltbd seems like a terribly cryptic name.  can you come up with something
better?  also it looks as if mMltbd will be uninitialized if you fail to
access the prefs service.  and, you should not use nsIPref in new code.  use
nsIPrefService/nsIPrefBranch instead.

> NS_IMETHODIMP nsIDNService::ConvertUTF8toACE(const char *input, char **_retval)
> {
>+  nsAutoString ustr;
>+
>+  ustr.Assign(NS_ConvertUTF8toUCS2(input));

replace this with the following:

    NS_ConvertUTF8toUCS2 ustr(input);

doing so eliminates one buffer copy.


>+      ace.Append(encodedBuf);
>+      ace.Append('.');

nit: 
	ace.Append(encodedBuf + NS_LITERAL_CSTRING("."));

each time you call Append on a string you are potentially calling malloc,
so it is worthwhile to coalesce calls to Append in this manner especially
from within a loop.


>+  *_retval = ToNewCString(ace);

the methods on nsIIDNService should actually take AUTF8String and ACString
types instead of "string" types.  as it currently exists the interface is
not scriptable because it uses "string" to represent UTF8 data.  xpconnect
will fail to convert UCS2 to UTF8 before calling ConvertUTF8toACE.  of
course the methods are currently marked |noscript| because of this fact.
we should file a bug to fix the interface or if you have time it would be
great if you could fix the interface now.  doing so would also cut down on
the number of buffer copies since you should not have to call ToNewCString
at the end of each method.

here's what the interface should look like:

interface nsIIDNService : nsISupports
{
  ACString    convertUTF8toACE(in AUTF8String input);
  AUTF8String convertACEtoUTF8(in ACString input);
};

also, we probably need to have a method on this interface to test whether
an ASCII string should be converted to UTF8 using convertACEtoUTF8.  this
method would be called by nsStandardURL in order to provide UTF8 to the
caller of nsIURI::GetHost.


>+static nsresult punycode(const char* prefix, const nsAString& in, nsACString& out)
...
>+  out.Assign(prefix);
>+  out.Append(encodedBuf);

nit:
    out.Assign(nsDependentCString(prefix) + nsDependentCString(encodedBuf));


>+static nsresult encodeToRACE(const char* prefix, const nsAString& in, nsACString& out)
...
>+  out.Assign(prefix);
>+  out.Append(encodedBuf);

nit:
    out.Assign(nsDependentCString(prefix) + nsDependentCString(encodedBuf));


>+nsresult nsIDNService::encodeToACE(const nsAString& in, nsACString& out)
>+{
>+  // RACE encode is supported for existing testing environment
>+  if (!strcmp("bq--", mACEPrefix))
>+    return encodeToRACE(mACEPrefix, in, out);
>+  // else do punycoce
>+  else
>+    return punycode(mACEPrefix, in, out);
>+}

else after return statement is meaningless.  do this instead:

 {
   if (...)
     return encodeToRACE(...);

   return punycode(...);
 }



>+
>+nsresult nsIDNService::stringPrepAndACE(const nsAString& in, nsACString& out)
>+{
>+  nsresult rv = NS_OK;
>+
>+  out.Truncate();
>+
>+  if (in.Length() >= kMaxDNSNodeLen) {
>+    NS_ERROR("IDN node too large");
>+    return NS_ERROR_FAILURE;
>+  }
>+
>+  if (IsASCII(in))
>+    out.Append(NS_LossyConvertUCS2toASCII(in));

nit:
      CopyASCIItoUCS2(in, out);

this is better since it eliminates the intermediate conversion buffer.


>+    nsCAutoString encodedBuf;
>+    rv = encodeToACE(strPrep, encodedBuf);
>+    out.Append(encodedBuf);

why don't you just do this:

      rv = encodeToACE(strPrep, out);

thereby eliminating the intermediate buffer "encodedBuf"
Comment 51 nhottanscp 2003-01-09 14:02:53 PST
>+CSRCS     = race.c  \
>+            nameprep.c \
>+            punycode.c

> so these files are going to be added to necko as well?

yes nameprep.c implements RFC 3454 and punycode is going to be a standard,
race.c can be removed after punycode becomes standard

>the methods on nsIIDNService should actually take AUTF8String and ACString
>types instead of "string" types. 

let me do the change separately (filed as bug 188410)


Comment 52 nhottanscp 2003-01-09 14:05:05 PST
Created attachment 111093 [details] [diff] [review]
xpcom glue code (address reviewer's comments)
Comment 53 nhottanscp 2003-01-09 14:20:52 PST
Created attachment 111097 [details] [diff] [review]
JPNIC code based on idnkit-1.0pr2 (added comment about the mapping table)
Comment 54 Darin Fisher 2003-01-10 02:04:40 PST
Comment on attachment 111093 [details] [diff] [review]
xpcom glue code (address reviewer's comments)

>Index: nsIDNService.cpp

> nsIDNService::nsIDNService()
...
>+      if (NS_SUCCEEDED(rv))
>+        mMultilingualTestBed = value;

mMultilingualTestBed will be uninitialized if, for example,
the pref service is not available.  shouldn't you at least
initialize it to some sane value?

fix that and sr=darin
Comment 55 nhottanscp 2003-01-10 10:11:44 PST
Comment on attachment 111097 [details] [diff] [review]
JPNIC code based on idnkit-1.0pr2 (added comment about the mapping table)

This is from idnkit-1.0pr2.
http://www.nic.ad.jp/ja/idn/mdnkit/download/

punycode is from the internet draft
http://www.ietf.org/internet-drafts/draft-ietf-idn-punycode-03.txt
Comment 56 nhottanscp 2003-01-10 10:14:27 PST
Comment on attachment 110982 [details] [diff] [review]
JPNIC nameprep data from idnkit-1.0pr2

This is from idnkit-1.0pr2, mapping table for RFC 3454,
http://www.nic.ad.jp/ja/idn/mdnkit/download/
Comment 57 Darin Fisher 2003-01-13 11:31:56 PST
Comment on attachment 110982 [details] [diff] [review]
JPNIC nameprep data from idnkit-1.0pr2

sr=darin
Comment 58 Darin Fisher 2003-01-13 11:33:08 PST
Comment on attachment 111097 [details] [diff] [review]
JPNIC code based on idnkit-1.0pr2 (added comment about the mapping table)

sr=darin
Comment 59 Darin Fisher 2003-01-13 11:35:26 PST
by how much does this IDN support code bloat necko.dll?  given that IDN is not
all that common, it might be worth it to break this out into its own library.
Comment 60 nhottanscp 2003-01-13 13:10:38 PST
In today's debug build on my machine, necko.dll is 1321k without the patch and
1365k after the patch.

I am going to check in the JPNIC code without hooking up to the build. The glue
code has to wait for Unicode normalizer (bug 8275).
Comment 61 Darin Fisher 2003-01-13 13:35:29 PST
nhotta: the size change in an optimized build would be more interesting.  since
a good portion of the increase due to this patch is the large nameprep table, it
stands to reason that much of the 45k increase would be transfered to an
optimized build.  and, last i checked the optimized size of necko.dll is roughly
450k, so this change potentially increases the size of necko.dll by 10%.  if
true, then that is not a trivial increase.  perhaps it might make sense to move
the nsIDNService into necko2.dll, which contains the less frequently used
networking code.  at any rate, i just think this is something that should be
investigated.
Comment 62 nhottanscp 2003-01-13 14:27:43 PST
>perhaps it might make sense to move the nsIDNService into necko2.dll, 
>which contains the less frequently used networking code.
I think that make sense at least for now.
Comment 63 nhottanscp 2003-01-13 14:44:32 PST
darin, how can we detach nsIDNService from necko.dll?
Do we need to make a new directory like netwerk/idn and move the files from
netwerk/dns because we first need to detach it from neckodns_s.lib?
Comment 64 Darin Fisher 2003-01-13 14:48:58 PST
nhotta: right something like that would be required.  but before going to all
that trouble, can you determine if this is even necessary?  i'm only suggesting
that it might be a good idea, but without actually comparing optimized builds
it's nearly impossible to know for sure.
Comment 65 nhottanscp 2003-01-13 17:00:04 PST
I disabled debug, ac_add_options --disable-debug.
necko.dll without IDN is 696k
necko.dll with IDN is 732k

so, difference is 36k. 
Comment 66 Alec Flett 2003-01-13 17:17:37 PST
don't forget making a new DLL has the 10k disk / 32k runtime footprint overhead.
perhaps this belongs in necko2 so we don't take the hit of another dll?
Comment 67 nhottanscp 2003-01-14 14:47:43 PST
alec, the current patch is adding to necko.dll which increase 36k for non debug
build (see comment #65). Do you think that is okay? Or do we want to put IDN to
necko2.dll?
Comment 68 Alec Flett 2003-01-14 15:25:25 PST
I don't know enough about how much IDN is used to know where we should put it -
necko2 is no longer in the default embedding client (though embeddors are still
welcome to add it themselves) - so the question is, is it safe to leave out of
the default embedding builds?
Comment 69 nhottanscp 2003-01-14 15:58:07 PST
IDN has to be included as default. It is going to be a standard soon.
Let me proceed with the current approach. We can do separate dll later if 36k is
a problem and the change should be a mechanical one.
Comment 70 Darin Fisher 2003-01-14 16:32:59 PST
nhotta: did your non-debug build include the "--enable-optimize" flag?  if not,
then 36k might be an overestimate which is good :)

i'm starting to agree more and more with just keeping this in necko.dll.  let's
go for that for now.
Comment 71 nhottanscp 2003-01-15 10:00:43 PST
>nhotta: did your non-debug build include the "--enable-optimize" flag?
No, I didn't. With that flag,
necko.dll without IDN is 428k
necko.dll with IDN is 460k

32k is added by IDN
Comment 72 R.K.Aa. 2003-01-15 19:14:36 PST
it isn't many hours since i built from CVS, and after re-building now, including
this checkin, i suddenly see HTTP headers in top of content area at 
http://www.aftenposten.no
Comment 73 R.K.Aa. 2003-01-15 19:22:59 PST
Please ignore comment 72 - people see this with older builds too. Site error i think
Comment 74 Stig Nygaard 2003-01-16 11:37:10 PST
Is this related to bug 157388, maybe even a duplicate? Or are the .nu domains
problems with letters like זרו etc. a different case?
Comment 75 Stig Nygaard 2003-01-16 11:40:04 PST
sorry for the previous spam. Actually I thought I was standing on another bug
when submitting. Maybe still a bit related, but this one seems very technical ;-)
Comment 76 nhottanscp 2003-01-21 10:59:21 PST
Created attachment 112169 [details]
instruction for testing
Comment 77 nhottanscp 2003-01-21 11:01:50 PST
checked in to the trunk on 1/17/2003
Comment 78 bobj 2003-03-10 10:20:10 PST
RFC update.  In addtion to:
  - stringprep: RFC 3454 (http://www.ietf.org/rfc/rfc3454.txt)

3 more RFCs have been finalized:

  - IDNA: RFC 3490 (http://www.ietf.org/rfc/rfc3490.txt)
  - nameprep: RFC 3491 (http://www.ietf.org/rfc/rfc3491.txt)
  - punycode: RFC 3492 (http://www.ietf.org/rfc/rfc3492.txt)
Comment 79 Yuying Long 2003-03-10 11:16:52 PST
Mark as verified since IDN works on trunk build by following the instruction in
comment #76, and the we have bugs for remain problems.

Note You need to log in before you can comment on or make changes to this bug.