Closed
Bug 215212
Opened 21 years ago
Closed 18 years ago
Chrome registry should not be used for core component i18n and l10n
Categories
(Core :: Internationalization, defect, P4)
Core
Internationalization
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: benjamin, Assigned: benjamin)
References
Details
Attachments
(1 file)
37.07 KB,
patch
|
darin.moz
:
review-
|
Details | Diff | Splinter Review |
Hyatt points out in bug 206358 that the chrome registry is used for localization
for core parts of gecko. This should really be handled by a much simpler
mechanism... right now I'm thinking of a simple resource://locale/ mapping
that's set to the current locale at startup...
Then we won't need the simplechromeregistry as part of the MINIMO build, becaue
netwerk/layout will not depend on a chrome implementation.
Comment 1•21 years ago
|
||
sounds like a great plan to me. is there enough data such that a jar file still
makes sense?
Summary: Chrome registry should not be used for core component i18n and l10n → Chrome registry should not be used for core component i18n and l10n
Assignee | ||
Comment 2•21 years ago
|
||
darin: I don't think a JAR file is necessary, especially considering that we
don't compress files in mozilla JAR packages anyway.
Right now I'm looking at all files currently installed to embed.jar.
Comment 3•21 years ago
|
||
I wonder if we want to make the resource: url handler understand jars. for
instance, if resource://foo/bar.properties doesn't exist, maybe it could try
jar:resource://foo.jar!bar.properties
Unfortunately, the chrome registry really plays two roles - one is to abstract
packages into actual file locations, and the other is to hide file locations
behind jar's.
I'm just typing as this occurs to me, but perhaps there is some way we could
abstract out XXX -> jar: mapping...
so from chrome:// urls this would look like:
- parse the chrome URL, and use current locale/skin information to transform it
into a resource:// URL
- transform the resource:// url into a .jar url (if necessary)
for resources, it would just
- transform the resource:// url into a .jar url (if necessary)
Comment 4•21 years ago
|
||
I should add that the point of .jar files is not to compress the files.. the
point is improved performance because you're not opening up eight million file
descriptors. When you're basically just streaming in a whole bunch of data that
will probably be read once (because, for instance, the .properties files are
cached in memory after they've been read in)
Its a bit like combining multiple .dlls into one.
Assignee | ||
Comment 5•21 years ago
|
||
Actually, the resource:// handler can already understand JARs, so that's not a
problem. I see how a JAR might be more efficient than flat files, so I can do it
that way if it seems better.
Assignee | ||
Comment 6•21 years ago
|
||
This provides the interface nsIGeckoLocale and the resource://gecko-locale/
mapping into a JAR file. This jar will live in [gre-dir]/res/locales/
This patch also includes
1) a crash-fix in the nsCOMPtr-helper for the category manager
2) moving the frozen nsIObserverService contractid #define into nsXPCOMCID.h
3) a category so that resource: protocol mappings can auto-instantiate as
needed
Assignee | ||
Comment 7•21 years ago
|
||
Comment on attachment 139137 [details] [diff] [review]
nsIGeckoLocale
darin: could you review the major code changes here, and I'll let bryner review
the rest of the changes for this bug (build-config stuff and jar-restribution).
Attachment #139137 -
Flags: review?(darin)
Comment 8•21 years ago
|
||
ok, here's my review comments:
1) why does nsIGeckoLocale live in necko? it does not seem to belong there
since it has nothing really to do with necko. the fact that there is a
resource://locale/ mapping related to nsIGeckoLocale does not seem to justify it
since nsIResProtocolHandler provides the necessary APIs. nsIGeckoLocale should
probably live in intl along with the other "locale" interfaces. why wasn't that
location chosen?
2) what's going on with the nsIGenericFactory changes?
3) is nsIGeckoLocale the best name for this interface? why "Gecko"-Locale?
isn't it bad to use a product name in an interface name? what if that product
name should change for some reason? "phoenix->firebird->blah" what if gecko
gets challenged and we have to change it? better to avoid product names in
interfaces IMO :-/
4) check spelling error(s) in nsIGeckoLocale
i really like the category manager solution for lazily loading substitutions.
but what can someone do if they need to override a default substitution? there
is no category for that. maybe it doesn't matter (for now at least).
Comment 9•21 years ago
|
||
... and check the comments in nsGeckoLocale::Init.
Assignee | ||
Comment 10•21 years ago
|
||
> 1) why does nsIGeckoLocale live in necko? [snip] nsIGeckoLocale should
> probably live in intl along with the other "locale" interfaces. why wasn't
> that location chosen?
Because the intl/locale interfaces are a different kind of beast; they deal with
OS locales and codepages and such, and not with the UI language of mozilla. (I
discussed it briefly with smontagu/jshin). The reason I have it in necko is
because necko is the "lowest-level" module that currently depends on localized
chrome: URIs which need to be moved over to the resource://gecko-locale/ URI
scheme. If you don't want it in necko, and jshin doesn't want it in intl/locale,
can you suggest somewhere else?
2) what's going on with the nsIGenericFactory changes?
In several patches I have flying around, I have static x::Create x::Register
x::Unregister functions. It seems to me that a class-level Create function is
better than a global ClassConstructor function in general, because then you can
make the class constructor and Init function private, and you don't have to
define the constructor and registerproc's in the module file. These macros just
copy the ClassConstructor macros for use with static classmember functions.
3) is nsIGeckoLocale the best name for this interface? why "Gecko"-Locale?
isn't it bad to use a product name in an interface name?
I'm open to suggestions... this interface, of course, is only for core-gecko
l10n (necko/dom/layout), so XUL apps are still localized by the chrome registry.
Perhaps an interface name nsIUILocale with a contractID
@mozilla.org/gecko-locale;1 would be sufficient to differentiate the two cases.
4) check spelling error(s) in nsIGeckoLocale
I will repost a patch when I hear back about interface-name and impl location.
Assignee | ||
Updated•21 years ago
|
Priority: -- → P1
Comment 11•21 years ago
|
||
I just want to voice my similar objection to the title nsIGeckoLocale. What
about nsIApplicationLocale or something?
one minor comment:
+ if (NS_SUCCEEDED(SetLocale(localeFile, leafNameC))) return NS_OK;
put this on 2 lines so we can break on the return when debugging.
overall, I see where you're going with this , but this really seems like this is
an application or GRE-level interface, higher than necko. Does necko really
depend on this? I'll read the patch more but that seems kind of scary. Necko
shouldn't be responsible for this kind of application-level locale smarts.
Assignee | ||
Comment 12•21 years ago
|
||
alecf: this is a GRE-level interface, but necko will use it (indirectly) because
it needs to load localized error messages from a stringbundle. We currently do
this through chrome://necko/locale/ URIS (see
http://lxr.mozilla.org/mozilla/source/netwerk/base/public/netCore.h#44) and I'm
trying to move all of "core gecko" to a simpler non-chrome mechanism.
Other modules that need converting are:
intl (charsetTitles.properties and global-strres.properties)
DOM (dom.properties)
accessibility (accessible.properties)
CAPS (caps.properties)
content/layout (various crap)
parts of embedding (nsWebBrowserPersist.properties)
parser (xmlparser.properties)
I'm defining "core gecko" as "stuff needed for minimo" (HTML rendering,
--disable-xul) such that minimo won't have to ship a chrome registry at all. So
stuff like printDialog.properties and xpinstall.properties won't be converted,
because they are only used by XUL.
Comment 13•21 years ago
|
||
> this is a GRE-level interface, but necko will use it (indirectly)
by this you mean, via a resource:// URL where the resource key has been setup by
someone using nsIResProtocolHandler, right? i don't see why necko will ever
need to interact with nsIGeckoLocale directly. can you explain that bit?
Comment 14•21 years ago
|
||
From the point of view of learnability and uptake
it is confusing to see the term Gecko floating around
inside Necko. One more vote here to change that.
It would similarly be less confusing if "Locale" wasn't
overloaded to mean both "Mozilla Locale" and "Operating
System Locale". Can there be another term for the OS variety?
Finally, forgive my total ignorance, but are resource: and
jar: not URL schemes, and assumedly therefore the province
of Necko in terms of retrieval? Why isn't the localisation
required for Gecko to be factored out into a lower level
library so that Gecko can operate without URLs if necessary?
Sorry, trying to catch up here.
- N.
Assignee | ||
Comment 15•21 years ago
|
||
Nigel: don't spam my bugs
Darin, alecf: I don't really care *where* this interface/impl lives, or what the
name is. embedding/components crossed my mind, however, that creates a little
nightmare with build dependencies (parts of embedding require profile, and
profile is gonna require this interface), and I'm gonna have to reorder
rdf/chrome to be after this interface.
I don't think it ought to live in profile/ because I'm hoping to move most of
profile/ out of the GRE.
Since 1.7a has been pushed back at least two weeks, I'm hoping that I can get
this in before then, although I need to coordinate some other stuff (xpinstall
needs to have a GRE path API, and localizers might want a little time to adapt).
Comment 16•21 years ago
|
||
Drive-by nit: the new code in the patch tests NS_SUCCEEDED and nests ifs,
marching the "normal" code path ever right-ward. How about following usual
style and returning rv early if (NS_FAILED(rv)), instead?
The trick with an addition like this is to name it well in a home where the name
fits. Just because Necko is the lowest-level module that might indirectly
depend on this new API doesn't mean it belongs there. It could go some place
even more low-level. What are the direct dependencies on it likely to be,
ultimately? I mean, who all will include the interface .h file?
/be
Comment 17•21 years ago
|
||
Re comment 15. *bristle* I care very much about names.
It makes Mozilla worse overall and my Mozilla contribution harder
to make if I have to constantly gloss over or deconstruct poorly
chosen terminology. You don't have to apologise in public print
to readers for bad naming, I have to apologise for you.
An ounce of prevention please.
- N.
Assignee | ||
Comment 18•21 years ago
|
||
profile, chrome, and probably parts of embedding are going to depend on this
interface. The interface itself depends on netwerk.
brendan: you are much better at naming/locations than I; I welcome suggestions.
The ever-rightward-if syntax was inspired by darin's codesize measurements for
functions with early returns and nsCOMPtrs and strings on the stack. Darin used
goto and saved some significant bloat. I thought the nested-if approach was a
bit prettier; I am already unhappy with how much code I had to write to
accomplish something this simple.
Comment 19•21 years ago
|
||
hmm... heres something I've wanted to do forever - fork rdf/chrome into a new
chrome implementation that doesn't depend on RDF. Maybe just a pipe dream, but
its the reason I added the -p switch to regchrome
Comment 20•21 years ago
|
||
I don't get the aversion to RDF. The chrome registry was always supposed to be used only for chrome,
i.e., for XUL. That it was used by Gecko was a mistake on the part of the people who did that, and that
mistake is - ideally - slowly being corrected.
When talking about the XUL chrome registry, I fail to see how removing RDF really improves things.
Before you can start up a XUL-based app, you still have to load an XML parser, Necko, RDF (for XUL
templates), etc., so the current chrome registry doesn't incur any additional cost to startup/footprint in
a XUL app. However to invent some new scheme for doing aggregation/superimposition and some new
text file format for parsing would add additional overhead, so I fail to see the benefit of switching away
from RDF.
The current chrome registry only makes no sense if you try to use it in an embedding context, which
was of course just bad design/abuse on the part of the people who did that in Gecko originally.
Comment 21•21 years ago
|
||
well, my objection is that the chrome registry is inexorably tied to the jar
packaging system. Maybe I should have been more clear. I want jar packaging in
an embedded context...
Assignee | ||
Comment 22•21 years ago
|
||
alecf: maybe I should post the other half of this patch, even though it's not
finished. I'm definitely using JAR packaging... right now, just for locales...
I'm also planning on permanently repackaging the files listed embed-jar.mn into
a non-chromeregistry content JAR file, as a separate bug.
Comment 23•21 years ago
|
||
Comment on attachment 139137 [details] [diff] [review]
nsIGeckoLocale
i think we agreed to move nsIGeckoLocale into embedding/components.
Attachment #139137 -
Flags: review?(darin) → review-
Comment 24•21 years ago
|
||
There is a bug #230596, which relates to this one, but deals with the separation
of the help-files into a separate package. Help wanted.
Assignee | ||
Comment 25•21 years ago
|
||
status update: I moved the impl, now called nsEmbedLocale, into
embedding/components. I have a tree which works, but I'm having all kinds of
interesting problems with the bootstrap sequence between the profile startup
code and the chrome registry. Pushing this out to beta.
Target Milestone: --- → mozilla1.7beta
Assignee | ||
Updated•20 years ago
|
Priority: P1 → P4
Target Milestone: mozilla1.7beta → ---
Assignee | ||
Comment 26•18 years ago
|
||
Now that the (new) chrome registry has much improved performance and no RDF overhead, I think we can stick to it instead of creating a separate service. We still need to solve some issues with multi-locale apps and selecting the system locale, etc.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•