Re: Review of Bi-Di code, bug 62777

[Erik van der Poel]

Simon Montagu wrote:
> 
> > From jst:
> >
> >In nsHTMLDocument.cpp there's this line that looks wrong
> >
> >+  if (charset.RFind("864", PR_TRUE )) {
> >
> >First of all, why not use the faster Find() in stead of RFind()? And why
> >do an expensive case-insensitive search when you're looking for numbers?
> 
> I accept your other comments, but I think the reason for using RFind
> is that if the substring "864" is present in the charset name, it will
> always be at or near the end. (ibm864 or ibm864i)

I don't like this at all. If we ever introduce any new charset name with
the substring "864" in the future, this code will cause problems. Please
spell out the whole name.

But, wait...

Why is it looking at the charset name in nsHTMLDocument.cpp anyway? It
looks like you're changing the charset to IBM864i if the bidi pref is
set to logical. This seems really strange to me. What if the user is
trying to force the IBM864 charset using the menu? Will this pref code
disallow the override? Also, why is it in nsHTMLDocument, and not in
nsDocument? What happens to XML documents?

Erik