394315 - Source L10n for ChatZilla (work with CVS localization)

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[Robert Kaiser]

For shipping ChatZilla with localized versions of SeaMonkey, it would be good to have ChatZilla localizations in CVS, just like the SeaMonkey L10n itself once bug 286110 is resolved.
We can even enabled building different-language ChatZilla XPI packages or possibly multi-language ones, but the first step is to leave XPI generation en-US-only and enable us to build other languages in the SeaMonkey build process.

The biggest and probably most controversial step for this is that we need to move all localizable content of ChatZilla into a directory named "locales/", ideally extensions/irc/locales/
Reporter has the infrastructure in place even if it ships it locale in ab_CD.jar and not in the reporter extension itself, the suite/debugQA/ extension has the infrastructure but forces the build to en-US in its Makefile, which is probably what we want to do for ChatZilla at first as well, before we set some fallback there that uses the build locale and only falls back to en-US if the locale has no ChatZilla L10n.

I don't want to force all the work on you, I'll try to help myself with getting that going - including keeping the XPI scripts working with that change.
BTW, with what versions of what products is ChatZilla trying to stay compatible? We might be able to simplify some overlay/chrome logic if the only supported product based on the old chrome registry is the suite.
Also, is compatibility with products older than trunk only required for the XPI or even for version built with the Makefiles?

Comment 1

[:Gijs (he/him)]

I don't think backwards compatibility has anything to do with this issue. The resulting XPIs should be the same, no?


So then, we can safely worry only about the two/three/four ways we currently have of building ChatZilla:

1) Build using the makexpi.sh script. This uses the jar.mn file(s). AIUI, This would need to be updated to use the extra jar.mn file. We use the standard perl tools to do the parsing and jar-ing for us, so I presume this would not be hard to keep working.

2) Build as an extension for SeaMonkey trunk (we never have been able to build with Firefox this way, I believe, and I don't think now is a good moment to change that - even if we ignore the Bug/Feature debate). This needs the same jar.mn changes, I presume, and possibly Makefile.in changes. As this process is the only thing that ever looks at the makefile, this should not be hard either.

3) Build a locale only extension using the makelocalexpi.sh script. As far as I'm concerned this could be a followup bug, ASSI to me (as I came up with this thing myself anyway).

4) Build as a XULRunner app. This is not officially supported, and is not in the tree. Rob Marshall has a patch for it, which we don't check in until xulrunner matures a bit more as a framework (at least, that's my understanding, Rob or James please scream if I'm saying Very Bad Things). This too is not really a concern for this bug, I believe - the patch may have some conflicts, but should otherwise continue to work fine.

CC-ing James, Samuel and Rob to make sure they're in on this debate.

Other than that, I'd be curious what a patch would look like.

Comment 2

[Robert Kaiser]

Attachment: cvs copies for chatzilla L10n

Here are the cvs copies needed for this, patch will follow in a minute.

Comment 3

[James Ross]

I assume the moves are because the "infrastructure" is too restrictive to actually work with what we've already got?

FWIW, the makexpi.sh script ultimately just does the same things the makefiles do, just without needing all the crap to actually use the makefiles.

Currently supported host applications are:
 - Mozilla Suite 1.0, 1.4, 1.7
 - Firefox 1.0, 1.5, 2.0, trunk
 - XULRunner 1.8.1, trunk

I don't care about SeaMonkey compatibility.

Comment 4

[Robert Kaiser]

Attachment: patch, v1: put infrastructure in place for source L10n (checked in)

Here's the patch. This makes ChatZilla work with the infrastructure for source L10n, but not actually use other languages than en-US yet.
The forcing to en-US is just there because for the Makefile-based approach, we need to put a more or less intelligent fallback at exactly the place where the forcing to en-US happens right now, so that localizers can but don't need to localize chatzilla when they want to get a working SeaMonkey in their language.
The makexpi.sh side of things is prepared with this to build a localized chatzilla, with the exception that it does not have the logic yet to find out where the files live for non-en-US languages.

contents.rdf for the locale is generalized so that localizers don't need to mess around with it and potentially break it, having the real locale files is enough for them.
makexpi needs to add the -c parameter to make-jars.pl that tells it where the localized files are, but that's not all: it needs to set the defines the contents.rdf preprocessing needs (after -- on make-jars), and this jar.mn files also needs preprocessing. As safeCommand doesn't support piping the preprocessed content to make-jars like rules.mk does, we need to create an intermediate file for this.
Oh, and I realized that the replacement for @MOZILLA_VERSION@ was superfluous on the contents.rdf files, so I removed it while I was at it.

Comment 5

[Robert Kaiser]

James:
Yes, the %-magic assumes that the US English version is in a locales/en-US directory and the jar.mn in the locales/ dir.
And actually, makexpi.sh simplifies some calls that are made with the Makefile stuff in the normal build system (esp. when it comes to preprocessing) and it complicates some stuff with the safeCommand function, which both made me go through some pain to find how to make this work - but then, you're probably enjoy my pain, as I probably was a bit rude to you lately ;-)
But actually, while we might not agree about backwards compat issues, we both want to improve this product and functionality, so let's be nice to each other, ok? :)

Comment 6

[James Ross]

makexpi.sh does the minimum work required, safeCommand was a fun development to deal with reporting failures without always printing everything to the console.

I don't like the CVS moves (on principal), but that sounds like a non-negotiable issue. :)

The patch generally looks OK, although the removal of some attributes for "locale information" in the contents.rdf sets my appcompat warnings off - that'll need investigating.

Comment 7

[Robert Kaiser]

That information should never have been actually needed, that's why I removed it. As far back as I can remember, we always have used those fields anywhere just from the contents.rdf/chrome.rdf information of th "global" package.

Comment 8

[:Gijs (he/him)]

Comment on attachment 278999 [details] [diff] [review]
patch, v1: put infrastructure in place for source L10n (checked in)

<snip>

>Index: mozilla/extensions/irc/locales/jar.mn
>+* locale/@AB_CD@/chatzilla/contents.rdf          (generic/chrome/contents.rdf)
>+  locale/@AB_CD@/chatzilla/chatzillaOverlay.dtd  (%chrome/chatzillaOverlay.dtd)

I think these double subdirectories are useless, but apparently everyone else has them, and once upon a time in a happy future we may indeed have locale files that are not part of chrome. And then, consistency is such a wonderful thing. So you can leave them, now that I've complained about them :-).

<snip>

>Index: mozilla/extensions/irc/locales/Makefile.in
>+# override UI locale with en-US so that we don't need to provide L10n yet

You mean, so that L10N doesn't have to localize ChatZilla yet? :-)

<snip>

>Index: mozilla/extensions/irc/locales/generic/chrome/contents.rdf
>   <!-- locale information -->
>-  <RDF:Description about="urn:mozilla:locale:en-US"
>-        chrome:displayName="English(US)"
>-        chrome:author="mozilla.org"
>-        chrome:name="en-US"
>-        chrome:previewURL="http://www.mozilla.org/locales/en-US.gif">

James already mentioned this, I'm not sure why we want to remove this. The previewURL, ok, given it's a 404 it seems pretty damn useless, but other than that...
If you're sure that there won't be problems, you can leave it, and if we get complaints we'll find you ;-).

>Index: mozilla/extensions/irc/xpi/makexpi.sh
>+# This is where the actual L10n files are found, below $LOCALEDIR for en-US, in l10n/ repository for other languages
>+$L10NDIR="$LOCALEDIR/$AB_CD"

Remove the $ in front, it breaks win32 and is not really nice sh anyway. :-(

> # Check setup.
> if ! [ -d "$FEDIR" ]; then
>   echo "ERROR: Base ChatZilla directory (FEDIR) not found."
>   exit 1
> fi
>+if ! [ -d "$LOCALEDIR" ]; then
>+  echo "ERROR: ChatZilla locales directory (LOCALEDIR) not found."
>+  exit 1
>+fi

Please remove this check. We already check for the actual locale directory, so I don't really see the use of this one, in particular because with the L10N support it might be completely unrelated to the CZ locale dir. We don't check for the xul/ or js/ directories either, we just assume that a proper CVS checkout was done (if not, hopefully other bits and pieces will (have) yell(ed)).

> <snip>

>+safeCommand $PERL make-jars.pl -v -z zip -p preprocessor.pl -s "$LOCALEDIR" -d "$JARROOT" -c "$LOCALEDIR/$AB_CD" -- "-DAB_CD=\"$AB_CD\" -DMOZILLA_LOCALE_VERSION=\"\"" '<' "$LOCALEDIR/jar.mn.pp"

I assume you really meant to use $L10NDIR here instead of $LOCALEDIR/$AB_CD ? If not, there is currently nowhere you actually use $L10NDIR, so then you might as well remove it.


with the above,
r=gijs

Comment 9

[Robert Kaiser]

(In reply to comment #8)
> >Index: mozilla/extensions/irc/locales/Makefile.in
> >+# override UI locale with en-US so that we don't need to provide L10n yet
> 
> You mean, so that L10N doesn't have to localize ChatZilla yet? :-)

I mean so that it's no must for localizers to provide an actual localization for ChatZilla yet - the possibility to do so will come in the next step (by testing exactly in this place if a CZ L10n does exist)

> >Index: mozilla/extensions/irc/locales/generic/chrome/contents.rdf
> >   <!-- locale information -->
> >-  <RDF:Description about="urn:mozilla:locale:en-US"
> >-        chrome:displayName="English(US)"
> >-        chrome:author="mozilla.org"
> >-        chrome:name="en-US"
> >-        chrome:previewURL="http://www.mozilla.org/locales/en-US.gif">
> 
> James already mentioned this, I'm not sure why we want to remove this. The
> previewURL, ok, given it's a 404 it seems pretty damn useless, but other than
> that...
> If you're sure that there won't be problems, you can leave it, and if we get
> complaints we'll find you ;-).

Well, we'd need to give the localizers a possibility to provide the right displayName - and actually nothing uses any of those properties for anything but the "global" chrome package (oh, and never has, since I can remember, and I was a localizer way before Mozilla 1.0).

> Remove the $ in front, it breaks win32 and is not really nice sh anyway. :-(

Yes, sorry, my fault.

> >+if ! [ -d "$LOCALEDIR" ]; then
> >+  echo "ERROR: ChatZilla locales directory (LOCALEDIR) not found."
> >+  exit 1
> >+fi
> 
> Please remove this check.

Right, it's probably too much.

> >+safeCommand $PERL make-jars.pl -v -z zip -p preprocessor.pl -s "$LOCALEDIR" -d "$JARROOT" -c "$LOCALEDIR/$AB_CD" -- "-DAB_CD=\"$AB_CD\" -DMOZILLA_LOCALE_VERSION=\"\"" '<' "$LOCALEDIR/jar.mn.pp"
> 
> I assume you really meant to use $L10NDIR here instead of $LOCALEDIR/$AB_CD ?

Oops, yes, forgot to use L10NDIR here.

Comment 10

[Robert Kaiser]

Comment on attachment 278999 [details] [diff] [review]
patch, v1: put infrastructure in place for source L10n (checked in)

OK, this part has landed.

The next step is to really make this working with a language other than en-US. I'll work on this using de for testing, as I actually have the localization for that around locally. I'll mostly care about getting it to work in the SeaMonkey build process, but I put the infrastructure in place so that I hope we can even get this for makexpi.sh

Comment 11

[Robert Kaiser]

Attachment: step 2, v1: use it in the SeaMonkey build process

This patch actually makes us use the localization in the SeaMonkey process. For this, we have all languages that support chatzilla L10n listed in our all-locales file. If the language the build system is currently building is not in that file, we fall back to en-US. Additionally, we are calling compare-locales so that we can detect and will list in the build log if the localization has missing strings.
I still need to investigate if we should list extensions/irc in the locale checkout variable in client.mk but that's not needed for this patch to work.

Comment 12

[Robert Kaiser]

Attachment: step 3, v1: use it in makexpi

Umm, it wasn't my intention originally to still do this tonight, but here it is.
This third patch needs the all-locales file from the second one and lets makexpi.sh generate localized chatzilla packages.
I made the packages have names of chatzilla-$AB_CD-$VERSION.xpi so that we can easily identify localized XPIs.

To test this, execute the following commands (German L10n for ChatZilla is already in the L10n CVS):
-----------
cvs -d :pserver:[e-mail]:/cvsroot co mozilla/config mozilla/extensions/irc
cvs -d :pserver:[e-mail]:/l10n co l10n/de/extensions/irc
cd mozilla/extensions/irc/xpi/
export AB_CD=de
sh makexpi.sh
-----------
...and you will get an installable German ChatZilla XPI :)

Comment 13

[:Gijs (he/him)]

Comment on attachment 279287 [details] [diff] [review]
step 3, v1: use it in makexpi

>Index: mozilla/extensions/irc/xpi/makexpi.sh

> # This is where the actual L10n files are found, below $LOCALEDIR for en-US, in l10n/ repository for other languages

Nit: now that I see this again, it'd be better to have this comment on 2 lines. Generally, CZ uses a line limit of 80 chars.

With that, r=gijs

Comment 14

[:Gijs (he/him)]

Comment on attachment 279287 [details] [diff] [review]
step 3, v1: use it in makexpi

Bugzilla needs to read minds.

Or at least comments.

Comment 15

[:Gijs (he/him)]

Comment on attachment 279282 [details] [diff] [review]
step 2, v1: use it in the SeaMonkey build process

r=me, but I'm not very good with makefiles. If you want to be sure, ask bsmedberg for an additional review.

Comment 16

[Robert Kaiser]

luser has sanity checked the Makefile change via IRC and said it was OK.

Checked in the cvs removal of the old locale files and steps 2+3, I consider this bug fixed.

We might still want to look into issues like if we need to add extensions/irc to LOCALES_suite in client.mk or such, and we might need to revisit some changes made here when we need to deal with SeaMonkey locale repackaging and automatic updates of localized builds, but we'll look into those when they come up.

Comment 17

[Axel Hecht [:Pike]]

Please clarify in .l10n on what's expected to happen with the chatzilla localization, we need a concrete agreement between the module owner and the localizers.

Comment 18

[:Gijs (he/him)]

I do like hearing this *after* the patches have landed.
</sarcasm>

In other news, according to despot rginda is the module owner and he's not getting any bugmail for the above comment as things are right now. Changing that...

You also didn't clarify what you actually expect. What kind of "concrete agreement" are we talking about? Agreement for the localizers to keep up with the ChatZilla trunk code? Agreement of the ChatZilla devs to not make locale changes between certain dates? Agreement to have weekly/fortnightly/monthly meetings? Which localizers/localizations? (all of the locales, or only some?) Does it need to be a legal agreement, or 'just' a newsgroup thread, or...?

Or, from a different perspective, what's still broken? What would the agreement be fixing?

Comment 19

[:Gijs (he/him)]

And now KaiRo didn't mention he already posted something, apparently without confirming anything in the post with any of the ChatZilla or l10n people (as far as I'm aware). Yay for general confusion.

http://groups.google.com/group/mozilla.dev.l10n/browse_thread/thread/98ae84acf4f2daa5/99a818bf7b6ff47b

Comment 20

[Axel Hecht [:Pike]]

Re comment 18, all of the above. Even not answering some of those questions should be a conscious decision.

The all-locales hack is cruft, really. And it requires a commitment of the add on developer to actually remove locales that are broken, and to re-enable them when they're fixed. Which kind-of turns down the idea of putting the localization into l10n in the first place.

On the other hand, not all localizations that may sucessfully compare may be ready to be included in a release at any time, so you can't just say "we'll take whatever is green".

The current setup seems to create a bunch of different extensions with the same version and the same extension id. That sounds like something the owner should sign up for, too. Not that I think it's the right thing to do. Maybe the l10n files should be packaged up as a dependent package.

Comment 21

[Robert Kaiser]

A dependent package, now that we AFAIK have the possibility to do it on trunk, might solve a number of potential problems.
Still, the XPI generated by makexpi.sh _needs_ to be compatible to all of 1.8 branch, maybe even 1.7 branch or older versions, which closes out dependent XPIs for that in the usual case.
But then, makexpi.sh and the extensions on add-on download sites is something that people care about manually so we might not need such strong confirmation than for in-build stuff, a simple QA run could be enough there in any case.

Comment 22

[Robert Kaiser]

The basic groundwork has been done here with the move of the files etc.
I'd like the further work on changing this to do dependent packages in a different bug report for more clarity. I filed bug 397246 for this.