Currently, nsIRDFResource is written under the assumption that URIs are always in ASCII. With IDNs(internationalized domain names), that's not the case any more. We have to replace 'string' with 'AUTF8String'. There are a number of 'customers' (callers, child classes/implementations) so that it's a significant amount of work.

Have patch, will travel.

Comment on attachment 143055 [details] [diff] [review] add nsIRDFResource.valueUTF8 >Index: mailnews/imap/src/nsImapMailFolder.cpp >- nsAutoString uri; >- uri.AppendWithConversion(mURI); >+ NS_ConvertUTF8toUTF16 uri(mURI); > uri.Append(PRUnichar('/')); > > uri.Append(*name); > char* uriStr = ToNewCString(uri); so this is very interesting.... you are taking care to treat mURI as UTF-8 when converting to UTF-16. however, the code calls ToNewCString which does a lossy conversion (assuming |uri| is ASCII). that doesn't seem ok to me. >+ nsAdoptingCString escapedName(nsEscape(mURI.get() + rootURI.Length(), >+ url_Path)); >+ if (escapedName.IsEmpty()) { >+ return NS_ERROR_OUT_OF_MEMORY; >+ } hmm.. are you sure |mURI.get() + rootURI.Length()| will never be an empty string? >Index: mailnews/news/src/nsNewsFolder.cpp >+#if 0 >+ nsAutoString sep; >+ rv = nsGetNewsFolderSeparator(sep); >+ if (NS_FAILED(rv)) return rv; > str += sep; >+#endif maybe ask a mailnews guy if he really wants to keep this old |#if 0|'d code?? the rest looks fine to me.

Updated with darin's nits. I added an assertion for the nsImapMailFolder::GetFolderURL, because it should always be a longer URI. I left in the #if 0 code, and will send mail hoping for an answer, but don't want to hold the patch for that. Pike: I think the interCaps naming is acceptable here, because this method is designed for scripting use; it's just a one-character change in the patch, though, so I'll do what you prefer.

(In reply to comment #4) > Pike: I think the interCaps naming is acceptable > here, because this method is designed for scripting use; it's just a > one-character change in the patch, though, so I'll do what you prefer. If I knew. We should probably have drivers vote on whether we ever want to move RDF to interCaps at all. Though we need a plan for that, and this bug is prolly a bit more urgent. How about adding a @note that we may change to LeadingCaps?

Side note: the general interCaps issue for the RDF code is in bug 189120.

Comment on attachment 143121 [details] [diff] [review] nsIRDFResource.valueUTF8 - for (const char* p = mURI; *p && *p != ':'; ++p) + for (const char* p = mURI.get(); *p && *p != ':'; ++p) contractID.Append(*p); This is not your yacky string code, so you don't have to fix it. Note to self, eventually someone should.

> + for (const char* p = mURI.get(); *p && *p != ':'; ++p) > contractID.Append(*p); how about this: PRInt32 i = mURI.FindChar(':'); if (i != kNotFound) contractID += StringHead(mURI, i - 1);

Comment on attachment 143121 [details] [diff] [review] nsIRDFResource.valueUTF8 my vote is for ValueUTF8 to be consistent with the rest of the RDF interfaces. "when in rome, stay in rome." does the RDF standard support IDN? if not, should we be doing this? i think it's reasonable, but i'm just asking ;-) > + nsCAutoString uri = mURI + NS_LITERAL_CSTRING('/'); does this compile? double quotes should be needed. sr=darin

checked in, with one win32 bustage fix in nsAbOutlookCard.cpp