485692 - Build fails if *.accesskey is specified in l10n

Status: NEW

(Other Applications :: ChatZilla, defect)

Description

[Atsushi Sakai]

Bug 474093 adds a feature that can specify accesskey with keys named *.accesskey.

But if *.accesskey is added in l10n, compare-locales.pl fails at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/irc/locales/Makefile.in&rev=1.6&mark=86#83
because it does not match with en-US strings.
Then, the build will fail.

So either of these is required:
- The build should success even if compare-locales.pl fails.
- All l10n, including en-US, should use *.accesskey.

Which way is better?

Comment 1

[James Ross]

None of the options are good. Ignoring compare-locales.pl failure has got to be risky for localisations; switching all locales to always use .accesskey is a lot of work for a minority of locales.

My personal favourite solution is fixing compare-locales.pl to know what it's doing, but I doubt that'd go down well.

Comment 2

[Atsushi Sakai]

Axel:
Should we modify compare-locales.pl like Bug 316674?
Could you give us a comment?

Comment 3

[Atsushi Sakai]

Attachment: Patch v1.0

Also fixes Bug 485691.

Comment 4

[Axel Hecht [:Pike]]

Moving back to chatzilla, compare-locales.pl isn't really a toolkit feature these days. We should probably remove it from there, as it's not gonna get back to maintainance, and is replaced by the python impl.

Review comments in a minute.

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 369961 [details] [diff] [review]
Patch v1.0

r-, I don't see anything venkman-related in the original patch.

I'd prefer for chatzilla to just grab a copy of compare-locales.pl and own that, if you really want to continue to use it.

In real life, I'd prefer to see this use the python version of compare-locales, not that I can come up with the patch to do that anytime soon. Mostly because there's no support for filter.py in compare-dirs, which is what you wanted here.

Comment 6

[Atsushi Sakai]

Attachment: Patch v2.0

- Copied compare-locales.pl over from toolkit/locales to irc/locales/.
- Removed Venkman stuff.

(In reply to comment #5)
> In real life, I'd prefer to see this use the python version of compare-locales,

Filed Bug 485871 for that as follow-up.

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 369971 [details] [diff] [review]
Patch v2.0

r=me with nits, you should probably drop all the exception handling for browser's region.properties. That hardcoding stuff was one of the primary reasons to drop this code, btw.

Comment 8

[Atsushi Sakai]

Attachment: Patch v2.1

Dropped all the exception handling for browser's region.properties.

Comment 9

[Robert Kaiser]

The original feature already calls for trouble, but the chatzilla/venkman string system is crap anyway, so stacking more hacks on top of the crazy hack it is is probably not worth my getting annoyed. And compare-locales.pl sucks about as much as any perl or shell script stuff we have in the tree, even if perl syntax is saner than that sucky python stuff, the usage patterns and code are the other way round.

Comment 10

[:Gijs (he/him)]

(In reply to comment #9)
> The original feature already calls for trouble, but the chatzilla/venkman
> string system is crap anyway, so stacking more hacks on top of the crazy hack
> it is is probably not worth my getting annoyed. And compare-locales.pl sucks
> about as much as any perl or shell script stuff we have in the tree, even if
> perl syntax is saner than that sucky python stuff, the usage patterns and code
> are the other way round.

Thanks for the constructive comment. If you have a good idea in terms of doing better than what this patch does, preferably one that does not involve all the localisers redoing everything, then I'm sure we would consider doing it. If not, it doesn't help to just complain about the system, especially not if you're just insulting it without backing up any of your rather bold claims.

Comment 11

[dynamis (Tomoya ASAI)]

This is real l10n blocker bug now.
Japanese l10n is already properly completed but due to this bug, our locale files are rejected for Seamonkey 2.0.1 (requesting blocking-seamonkey2.0.1).

We can release without these entities to avoid this bug temporary but they will be obviously incomplete ja/ja-JP-mac builds. We must fix this bug and release ja/ja-JP-mac builds with *.accesskey.
# note: need to fix not only for chatzilla but also for venkman

If you are against to the patch, please handle (or assign to some proper person) this bug as a seamonkey project leader.

Comment 12

[Robert Kaiser]

The SeaMonkey team can neither influence what keys are taken for ChatZilla, nor take any ChatZilla changes for 2.0.x anyhow to not break the string freeze, so not blocking any 2.0.x version on this.

The only way to get a localization into 2.0.x is to work around this in whatever way possible.

Comment 13

[:Gijs (he/him)]

So, any reason we couldn't use the python system, if Axel says that's getting fixed (maybe it is already?). Sounds better than copying more config files. We build w/ Python now anyway... We can get rid of perl! Hurray!

Comment 14

[:Gijs (he/him)]

(In reply to comment #13)
> So, any reason we couldn't use the python system, if Axel says that's getting
> fixed (maybe it is already?). Sounds better than copying more config files. We
> build w/ Python now anyway... We can get rid of perl! Hurray!

To be clear, that was a question for the seamonkey folks... it's your build system, sorta. :-)

Comment 15

[dynamis (Tomoya ASAI)]

(In reply to comment #12)
> The SeaMonkey team can neither influence what keys are taken for ChatZilla, nor
> take any ChatZilla changes for 2.0.x anyhow to not break the string freeze, so
> not blocking any 2.0.x version on this.

Robert, *.accesskey entities are defined as *optional* keys, not change/add/remove of entities set.
This does *not* break the streng freeze. Current chatzilla included in Seamonkey 2.0.x need *.accesskey entities for complete l10n of some locales.
# I believe not only for ja but also some other locale should use them.
# maybe just they don't know what they should for chatzilla/venkman

What this bug require is, just the build script should handle optional keys as optional. Why you think this bug will break the string freeze?

Comment 16

[James Ross]

Comment on attachment 369972 [details] [diff] [review]
Patch v2.1

If the Python version of compare-locales can cope with our strings, I'd rather not have Perl going into ChatZilla.