148180 - intl libraries depend on necko (unicharutil, locale, and uconv)

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P2)

Description

[Alec Flett]

ok, so I finally did some investigation to find out why all the i18n libraries
seem to depend on necko, and determined that they're all using necko just to
load a couple large .properties files into a nsIPersistentProperties object -
most of the code predates string bundles, and should be using string bundles
anyway.. both for easier code maintenance and for the bonuses that string
bundles provide, like caching and locale-awareness.

I fixed up unicharutil, uconv, and locale so that they no longer depend on
necko. chardet will probably have to depend on necko in order to do fancy
character detection on documents, and intl (string bundles) will continue to
depend on necko for the time being.

patch forthcoming.

Comment 1

[Alec Flett]

Attachment: break necko dependencies

yeah, this cleans us up some.

Comment 2

[Rui Xu]

Code issue, QA -> yokoyama.

Comment 3

[Alec Flett]

oops, reassign to myself

roy / jag - can I get an r=/sr= on this patch? thanks!

Comment 4

[Roy Yokoyama]

ftang should look at the patch.
ftang: Can you review?

cc ftang

Comment 5

[jag (Peter Annema)]

If you don't mind, could you change ::GetService to do_GetService etc. where you
touch them? I'll look again after you get r= on this.

Comment 6

[Alec Flett]

mozilla 1.1alpha already passed, just moving out to 1.1beta

Comment 7

[Alec Flett]

jag: I actually removed all instances but one, and that one I can't use
do_GetService, because its being stored in a global, raw pointer.

can you sr= this? Thanks...

Comment 8

[jag (Peter Annema)]

Comment on attachment 85631 [details] [diff] [review]
break necko dependencies

sr=jag

Comment 9

[Frank Tang]

Comment on attachment 85631 [details] [diff] [review]
break necko dependencies

The patch looks like not just remove the dependency of necko but some other
stuff. Can you break down rewrite into seperate bug/patch? I don't understand
moving some case conversion compress class has anything deal with necko at all.
(it could be a good thing to do but nothing to do with necko dependency,
right?) Also, what kind of test have you perform to prove your patch do no
evil?

Comment 10

[Alec Flett]

ok, I've tested the patch, by making sure that entities continue to work, etc...
as for the nsCompressedCharMap, I'll move that to another patch... new patch
coming up later today

Comment 11

[Alec Flett]

ok, so coming back to this - the patch contains changes to nsCompressedMap, but
those changes are localized to nsCaseConvertionImp2.cpp and
nsCaseConversionImp2.h - if I don't check those in, can I review this? I have
run the usual regression tests, etc. 

In addition, if this patch didn't work, all entities would be broken in my
build, and thus all localized strings in the UI... which they aren't.

Frank, can I get that review now?

Comment 12

[Frank Tang]

Comment on attachment 85631 [details] [diff] [review]
break necko dependencies

r=ftang

Comment 13

[Alec Flett]

cool. fix is in. thanks folks.

Comment 14

[Alec Flett]

oops, marking fixed this time.

Comment 15

[Roy Yokoyama]

alec: Thanks for your help. :)  I see no necko linked in the makefiles

However, I see MOZ_NECKO_UTIL_LIBS in \uconv\src\makefil.in and
\unicharutil\src\makefile.in
I lxr'ed for MOZ_NECKO_UTIL_LIBS and 
http://lxr.mozilla.org/seamonkey/search?string=MOZ_NECKO_UTIL_LIBS
and it looks though it's a linux platform specific and not defined.  
Is it depreciated? (note, I searched the moz.org and returns nothing)

darin/ftang? do u know what MOZ_NECKO_UTIL_LIBS is used for?

Comment 16

[Darin Fisher]

i have no idea what MOZ_NECKO_UTIL_LIBS is used for.

Comment 17

[Alec Flett]

I think its a throwback to the days of a necko utility library which contained
NS_NewURL and friends (i.e. now inline in nsNetUtil.h) - it doesn't resolve to
anything, so its pretty harmless.

http://lxr.mozilla.org/seamonkey/search?string=moz_necko_util_libs

Comment 18

[Roy Yokoyama]

agreed, marking as VERIFIED.
unicharutil, locale, and uconv don't have necko dependency.