457290 - Figure out a way to load localized DTD files

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Bug 394105 Comment #10 From  Smokey Ardisson (no bugmail - do not email)   2008-04-27 13:47:38 PDT  

[12:06am] ardissone: kreeger: what about dtd files?
[12:07am] kreeger: dtd files are fun?
[12:07am] ardissone: does your code not handle them?
[12:07am] kreeger: the localized string override thing?
[12:07am] ardissone: yeah
[12:08am] kreeger: no
[12:08am] kreeger: oh
[12:08am] kreeger: um for lookup?
[12:08am] ardissone: i don't know
[12:08am] kreeger: because it just get's the key from the string-bundle-service
and then asks our component if we have an override for it
[12:09am] kreeger: i would imagine it works for .properties and DTD...
[12:09am] ardissone: smorgan could only get it working for .properties
[12:09am] kreeger: I think we have some DTD's in Songbird.... let me check
[12:10am] kreeger: eh no just .properties
[12:10am] sauron:        https://bugzilla.mozilla.org/show_bug.cgi?id=394105#c3 
[12:10am] thebot: sauron: Bug 394105 nor, --, Camino2.0, [e-mail],
NEW, [l10n] localize Gecko chrome strings on the trunk
[12:10am] kreeger: let me look at |nsStringBundle|
[12:13am] kreeger: er
[12:13am] kreeger: ardissone: I think |nsIStringBundle| only reades .properties
[12:13am] kreeger: it'll probably take a core patch to wire up to whoever reads
the DTD files...
[12:14am] ardissone: awesome :/
[12:14am] encro left the chat room. (Quit: Low slung dancefloor dynamics)
[12:15am] ardissone: can someone just take Gecko out back behind the barn and
shoot it?
[12:15am] kreeger: ardissone: is there a specific DTD file we are having
trouble with?
[12:16am] ardissone: all of them?
[12:16am] kreeger: doh
[12:16am] kreeger: i'm guessing the old string stuff like the prompts are DTD?
[12:16am] kreeger: oh no
[12:16am] kreeger: I've seen those keys come through
[12:16am] ardissone: no, those are properties
[12:16am] ardissone: but stuff in netError are dtd, for example
[12:17am] kreeger: ahh
[12:17am] kreeger: for the no-site-exists XUL pages and sucH?
[12:17am] ardissone: mhm
[12:17am] ardissone: config.dtd
[12:17am] ardissone: for about :config
[12:17am] ardissone: DTDs are for xhtml-type content, i guess
[12:18am] kreeger: yeah
[12:18am] kreeger: those suck for us
[12:20am] kreeger: if the key gets mapped as a RDF XML source inside gecko -
that might be quite a problem for us :(
[12:22am] kreeger: <!ENTITY nssFailure2.title "Secure Connection Failed">
[12:22am] kreeger: can be easily mapped as <label value="&nssFailure2.title"
...
[12:23am] kreeger: which I assume runs through XUL somewhere to make the lookup
- but it's not easily findable ATM
[12:25am] philor: I think the lookup is just what it is in any XML using any
DTD - Expat parses the XUL, reads the DTD, and replaces entities with their
values
[12:25am] kreeger: yeah i bet
[12:25am] sauron: "yay" :/
[12:26am] philor: so I don't know what you could do to change what's used for
&nssFailure2.title; other than intercepting the chrome: URL resolution for the
DTD, and sending it off to get a different (but entire, not just one entry)
file
[12:39am] sauron: someone should Bug 394105 Comment in 394105, at any rate

Phil's last Bug 394105 Comment almost implies we should be able to do this (although we'd
have to use actual .dtd files instead of .strings files, but we could ship them
in .lprojs still) if the way the override system works is "intercept chrome://
url calls"; is it?

Bug 394105 Comment #11 From Stuart Morgan 2008-09-26 09:00:01 PDT

*** Bug 457238 has been marked as a duplicate of this bug. ***

Bug 394105 Comment #12 From Stuart Morgan 2008-09-26 09:04:59 PDT

Having thought for a minute after duping that bug... we may want to open a new
bug (blocking this one) for finding a solution for loading localized copies of
DTD strings, since we are bug 248160 really only did half of what it was
intended to cover.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #0)
> Phil's last Bug 394105 Comment almost implies we should be able to do this
> (although we'd have to use actual .dtd files instead of .strings files, but 
> we could ship them in .lprojs still) if the way the override system works is 
> "intercept chrome:// url calls"; is it?

It's not, at least not in that sense.

Is there an equivalent to the nsIStringBundleOverride service that handles .dtd files?

Comment 2

[Axel Hecht [:Pike]]

For localized XML, you'll need to feed DTDs into the parser for the chrome urls it loads. Anything else would require intercepting the parser at the level of expat, last I checked.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

In bug 394105 comment 22, Axel suggested using the ChromeML service as an override mechanism (to locate entire localized jars in our .lproj folders, actually), but that service is only available in Toolkit's chrome registry.  There doesn't seem to be a real analogue in rdf/chrome (there is some sort of override feature there, but I can't tell how/if it gets activated).

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

We'll see if we can figure out anything before freeze.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

No one's working on this, so kicking it from b4 (and, thus, 2.0).

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

For any release off of 1.9.x, we'll have the toolkit chrome registry, so we'll have ChromeML.

From mfinkle's response in http://groups.google.com/group/mozilla.dev.embedding/browse_thread/thread/f1c6d94785f5bba4/acd5d97fdf410ba1?#acd5d97fdf410ba1, it sounds like we'd have to implement stuff in AppDirServiceProvider to resolve the bundle's active lproj folder when "ChromeML" gets called, but I'm still not sure how we get the Gecko locale system to call ChromeML.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #6)
> it sounds like we'd have to implement stuff in AppDirServiceProvider to resolve
> the bundle's active lproj folder when "ChromeML" gets called

Perhaps code very similar to http://hg.mozilla.org/releases/mozilla-1.9.2/annotate/9d4aa3b682a8/toolkit/xre/nsXREDirProvider.cpp#l702 (except resolving to the active .lproj)

> but I'm still not sure how we get the Gecko locale system to call ChromeML.

"ChromeML" is the constant/short-form/folderTag for NS_CHROME_MANIFESTS_FILE_LIST (e.g., the counterpart of all the things in xpcom/io/nsAppDirectoryServiceDefs.h).  nsChromeRegistry::CheckForNewChrome looks in the directories returned for NS_CHROME_MANIFESTS_FILE_LIST for .manifest files, and then registers the chrome listed in the manifests.  I think.

Then to get the Gecko locale system to use our stuff, we'd have to register our chrome as overrides for all the locale packages.  Or something to that effect.

There are some examples of the usage of this in the tree; you can find them by searching for ChromeML and NS_CHROME_MANIFESTS_FILE_LIST.  Unfortunately, instead of nice examples, I find them inscrutable.  Particularly, nsIToolkitChromeRegistry.idl appears to define the key=folderTag relationship, but all most all usages define that themselves; are they providing their own chrome registry entirely?

Comment 8

[Stuart Morgan]

Attachment: Localized manifest dir loading (?)

This causes us to serve our own manifest list, containing Resources/foo.lproj/ (we can add a subdirectory to that if we want) and the original MacOS/chrome/. I know it's being called, because if I leave off the latter, it crashes and burns almost immediately :)

Beyond that I don't know enough to know if it's really working. Particularly I don't know whether the first one wins or the last one if there are collisions, so you'll probably want to avoid any entries that conflict between the two locations.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Localize relevant .dtd files

This patch, in conjunction with Stuart's, makes the relevant DTD files (and the two stubborn .properties files that are used but whose components cache their stringbundles before the stringbundleoverride service gets called) localizable.

1) Instead of landing additional separate copies of these files in English.lproj, the Makefile copies the files from the source tree (which for about half of them is geckochrome/ since we're already forking them for whatever reason).

I'm sure there's a better way to do the Makefile rules than using nsinstall and running it on every rebuild, but I'm not mento, and he's not here for consultation, and I couldn't figure it out.  But nsinstall works, so…

2) It's a bit weird for gecko.manifest to live in English.lproj, but I'm not really sure where a better place for it would be.  This file defines chrome overrides that make Gecko use the various files from the .lproj.

(Marcello: we'll want to make sure the l10n tools copy these new files into the localized versions, since they're not .strings or .nib files; it's been so long since I touched them--it was still AppleGlot!--that I don't remember what happens.)

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 507369 [details] [diff] [review]
Localized manifest dir loading (?)

Stuart, there's a major problem with this patch (one that made me think I was going insane until I figured out what was happening, thankfully):

For purposes of our profile .plists, the profile directory has changed to ~/Library/Mozilla/profiles/; all of the plists live loose in there.  Other symptoms I've tracked down: the Cache folder is created directly in the profile folder (if you use the mozProfileDir Info.plist key, or launch it directly) rather than in ~/Library/Caches/$(mozProfileDir), the IconCache folder is never created anywhere (and the Console yells about various site icon caching failures).

I suspect what happened is that when we changed from nsIDirectoryServiceProvider to nsIDirectoryServiceProvider2, we needed to define (or override) a new/additional or different constant, and in absence of them, Gecko is wreaking havoc on our profile setup (and CAMINO_PROFILE_DIR).

Comment 11

[Stuart Morgan]

Ooops, it's actually easier than that; I misunderstood the relationship between NS_IMPL_SUPPORTS and inheritence, so I thought declaring support for Foo2, which inherits from Foo, would imply support for Foo for purposes of do_QueryInterface. Turns out it doesn't so nothing in core thought it was okay to call GetFile.

The fix is to change the NS_IMPL_SUPPORTS line to:

NS_IMPL_ISUPPORTS2(AppDirServiceProvider,
                   nsIDirectoryServiceProvider,
                   nsIDirectoryServiceProvider2)

Comment 12

[Stuart Morgan]

Oh, I forgot to ask: do we want to make the directory Foo.lproj/chrome or somesuch, instead of dumping everything directly into the lproj folder? That would be easy enough to do.

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #12)
> Oh, I forgot to ask: do we want to make the directory Foo.lproj/chrome or
> somesuch, instead of dumping everything directly into the lproj folder? That
> would be easy enough to do.

It's just 8 files (9 including the manifest); we have far more than that in foo_properties.strings files (not to mention nibs) already, so I think loose is fine.

One semi-related question: How hard would it be to add support for $profile/chrome to the list of places to look for things?  The old rdf chrome registry included (and, iirc, created) that folder automatically via its own code, but the new toolkit one doesn't, so we lost support for being able to load any third-party Gecko stuff by other-than-hacking-the-bundle.

Comment 14

[Stuart Morgan]

Wouldn't that be NS_APP_USER_CHROME_DIR, rather than NS_CHROME_MANIFESTS_FILE_LIST? That's where core looks for, e.g., userContent.css. If this is the right key, it would be easy to add another directory though.

Let's do a follow-up bug for that, so we can move forward here while figuring that out.

Comment 15

[Stuart Morgan]

Comment on attachment 507786 [details] [diff] [review]
Localize relevant .dtd files

Why do we need to copy them at all, vs just having the project reference their location in the tree directly?

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #15)
> Comment on attachment 507786 [details] [diff] [review]
> Localize relevant .dtd files
> 
> Why do we need to copy them at all, vs just having the project reference their
> location in the tree directly?

[4:41pm] ardissone: smorgan: on bug 457290, we typically don't fish random files out of random locations in the source tree
[4:41pm] smorgan: Okay
[4:41pm] ardissone: (also, it keeps things consistent with all of our other Gecko l10n files)

Comment 17

[Stuart Morgan]

Attachment: Add lproj to chrome directory, v2

Respun to fix the issues mentioned above.

Comment 18

[Stuart Morgan]

Comment on attachment 507786 [details] [diff] [review]
Localize relevant .dtd files

I'm no mento, but I think we can cobble something better up anyway based on my weak Make-fu and some research. How about something like this:

GECKO_CHROME_FILES = <what you have now>
GECKO_CHROME_DEST_DIR = generated/resources/localized/English.lproj
GENERATED_GECKO_CHROME_FILES = $(patsubst %,$(GECKO_CHROME_DEST_DIR)/%,$(notdir $(GECKO_CHROME_FILES)))

GECKO_CHROME_SOURCE_DIRS = $(dir $(GECKO_CHROME_FILES))
vpath %.dtd $(GECKO_CHROME_SOURCE_DIRS)
vpath %.properties $(GECKO_CHROME_SOURCE_DIRS)

[...]


$(GENERATED_GECKO_CHROME_FILES): \
  GECKO_CHROME_DEST_DIR/%: %
    mkdir -p $(dir $@)
    cp -p -v $< $@

Then add GENERATED_GECKO_CHROME_FILES to the libs:: dependency list

I haven't tested all this, so it may need some tweaking. I'm not wild about having to use a search to find the sources when we've already listed them, but this at least gives us actual generation rules, so I think it's better. (Probably there's an even better way that didn't present itself.)

---
References:
http://www.gnu.org/software/make/manual/make.html#Text-Functions
http://www.gnu.org/software/make/manual/make.html#Selective-Search

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Localize relevant .dtd and .properties files, v2: with real targets!

This patch implements the solution in comment 18--which was only missing a $() around GECKO_CHROME_DEST_DIR!.  I tweaked a couple of the variable names and comments and also removed the -v from cp, since the command is already being echoed by make itself.

So we should be all good to go here now :)

Comment 20

[Stuart Morgan]

Comment on attachment 509576 [details] [diff] [review]
Localize relevant .dtd and .properties files, v2: with real targets!

sr=smorgan

Obviously we'll want to wait and land this with the other patch though :)

Comment 21

[Stuart Morgan]

Landed http://hg.mozilla.org/camino/rev/0941fc592d87 and http://hg.mozilla.org/camino/rev/ccdc534ef592

Leaving sr? for post-landing review.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

--> Stuart since he wrote the GeckXPCOM C++ code.

FYI, the -u argument to 'hg commit' lets you specify the patch author.

Comment 23

[Stuart Morgan]

Ah, good to know

Comment 24

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Also, either this or bug 634398 caused about a 15ms Ts regression on both Xserves (it's impossible to tell which due to the landing windows), though I guess that's to be expected with additional chrome registration and folder enumeration/creation.