The maximum version numbers for Firefox and Seamonkey are really really old in the mozilla trunk rdf files. For Firefox 35.* and Seamonkey 2.32.*. This seems to magically work with the main xpi suite builds but the optional language pack is reported as incompatible in localized suite builds.

I borrowed some code from calendar which worked for me doing an en-us and de suite build under Windows. I am unable to test it with Firefox. I put a workaround in if the suite version number could not be read.

Comment on attachment 8708697 [details] [diff] [review] 1240304.patch This breaks building with the standalone ./makexpi.py script. It's not clear to me why anything would break here, anyway. Both Firefox and SeaMonkey are supposed to ignore maxversions > gecko 2.0, unless AMO indicates otherwise (which it won't, for langpacks, because they aren't on AMO at all). Can you investigate what is actually broken about the langpacks?

>> Can you investigate what is actually broken about the langpacks? I am not that familar with the whole XPI package format but it looks ok to me. The install.js was recently removed from m-c and c-c. This might have broken the install. I think the version numbers are now checked. Bug 1210791 still needs to be fixed too. I will check if I find something and if not will wait for this bug to be fixed first. The interim solution there might not be good enough.

Just checked again. The main xpi package does not care about the version number. But the language package can not be installed if the max Seamonkey version number in the rdf is not at least the same as the Seamonkey with which you try to install it.

Screenshot when it installs fine and version matches

Screenshot with version mismatch. No further information is displayed.

(In reply to Frank-Rainer Grahl from comment #3) > >> Can you investigate what is actually broken about the langpacks? > > I am not that familar with the whole XPI package format but it looks ok to > me. The install.js was recently removed from m-c and c-c. This might have > broken the install. I think the version numbers are now checked. I don't think install.js has been in use for years now. Do you actually know when this broke?

>> Do you actually know when this broke? No sorry. Looking at \mozilla\toolkit\mozapps\extensions\internal\XPIProvider.jsm I see: const COMPATIBLE_BY_DEFAULT_TYPES = { extension: true, dictionary: true }; These are the only ones were the maxVersion seems to be ignored. The locale type is not in the list. Lots of changes in the backend recently. I suspect if my private localized build is not faulty (everything else works<g>) it has been a problem for some time already.

Looks like user_pref("extension.checkCompatibility.nightly", false); isn't ready for retirement yet. :-/

Tony, this doesn't work. The language pack does not have the internal type extension. It also occurs on all trees I built.

(In reply to Frank-Rainer Grahl from comment #10) > Created attachment 8709315 [details] > Errror with "extension.checkCompatibility.nightly" false > > Tony, this doesn't work. The language pack does not have the internal type > extension. It also occurs on all trees I built. Hm. IIUC that pref (which I use) applies to any kind of XPI add-ons. I know that for instance it does apply to complete themes. I haven't tested langpacks recently, but ISTR that the last time I tried, the add-ons manager would let me enable, say, a 2.41a1 Ukrainian langpack on a 2.42a1 build. But my Ukrainian guest of the time didn't like the result — or in any case she didn't like my AZERTY keyboard — so I went back to the compiled-in en-US. :-/ If the ChatZilla translations are different I wouldn't know about it.

The tree changed a lot in the last few months. I wouldn't wonder if one if the changes for signing or validation broke this. Maybe I should try one of Adrian Kallas latest l10n builds again and see if the language pack in it installs. FRG

P.S. Did you restart the browser after setting the pref? ISTR that it is one of those prefs which is checked only at startup. http://kb.mozillazine.org/Extensions.checkCompatibility doesn't mention this however.

(In reply to Frank-Rainer Grahl from comment #12) > The tree changed a lot in the last few months. I wouldn't wonder if one if > the changes for signing or validation broke this. Maybe I should try one of > Adrian Kallas latest l10n builds again and see if the language pack in it > installs. > > FRG You could. But the latest builds there are dated 2015-12-20. AFAICT the supported platforms are W32, W64, L32, L64 but not Mac.

I tied Adrians 2.41 DE build from 2015 and the official 2.39 DE Windows x86. Both didn't include or install the Chatzilla language pack so I still think it's broken :) Didn't see an error message either. They just weren't there. Downloading the DE Chatzilla language pack from AMO worked. FRG

The Seamonkey 2.40 build candidate also does not contain the de Chatzilla language pack: https://archive.mozilla.org/pub/seamonkey/candidates/2.40-candidates/build4/unsigned/win32/de/ I will file a separate bug for this. FRG

I still think the language files are broken when bundling the extension with Seamonkey so I created a Version 2 patch which fixes the makexpi.py problems with the first version. For this to work in a comm-central suite build Bug 1210791 also needs to be fixed. Standalone doesn't need this bugfix. Tested with en-us and de locale on Windows 7 x64.

Comment on attachment 8716635 [details] [diff] [review] Version 2 patch which allows the standalone makexpi.py to work Review of attachment 8716635 [details] [diff] [review]: ----------------------------------------------------------------- LGTM! ::: Makefile.in @@ +39,5 @@ > +endif > + > +FIREFOX_MAXVERSION := $(FIREFOX_VERSION) > +ifneq (a,$(findstring a,$(FIREFOX_VERSION))) > +FIREFOX_MAXVERSION := $(shell echo $(FIREFOX_VERSION) | sed 's|\(^[0-9]*\)\.\([0-9]*\).*|\1|' ).* Nit: it's not clear to me what the purpose of the second group here is. You don't seem to use it? Wouldn't: sed 's|^\([0-9]*\).*|\1|' work just as well? ::: locales/Makefile.in @@ +48,5 @@ > +endif > + > +FIREFOX_MAXVERSION := $(FIREFOX_VERSION) > +ifneq (a,$(findstring a,$(FIREFOX_VERSION))) > +FIREFOX_MAXVERSION := $(shell echo $(FIREFOX_VERSION) | sed 's|\(^[0-9]*\)\.\([0-9]*\).*|\1|' ).* Same regexp change here, please (assuming I'm not missing anything). @@ +80,5 @@ > @$(PERL) $(topsrcdir)/toolkit/locales/compare-locales.pl $(srcdir)/en-US $(LOCALE_SRCDIR) > + $(call py_action,preprocessor,$(DEFINES) $(ACDEFINES) \ > + $(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/defines.inc \ > + $(LOCALE_SRCDIR)/defines.inc \ > + $(srcdir)/generic/install.rdf -o $(DIST)/xpi-stage/$(XPI_NAME)/install.rdf) Isn't this a separate patch and/or bug 1210791? Either way, this should be reviewed by someone who knows about the (SeaMonkey) build system. Added r?Callek for that.

Comment on attachment 8716635 [details] [diff] [review] Version 2 patch which allows the standalone makexpi.py to work Review of attachment 8716635 [details] [diff] [review]: ----------------------------------------------------------------- ::: Makefile.in @@ +35,5 @@ > + > +# not a comm-central build > +ifeq (.*,$(SEAMONKEY_MAXVERSION)) > +SEAMONKEY_MAXVERSION := 2.41.* > +endif without comm/m-c theres no `configure`, and thus nothing to evaluate the Makefile.in down. I'd remove this block (and the related Firefox one) (I'm also not a fan of hardcoded magic numbers, especially ones we expect to change often) ::: locales/Makefile.in @@ +44,5 @@ > + > +# not a comm-central build > +ifeq (.*,$(SEAMONKEY_MAXVERSION)) > +SEAMONKEY_MAXVERSION := 2.41.* > +endif same here... in fact why not make this logic its own .mk that we use in both places, rather than duplicating the code? @@ +80,5 @@ > @$(PERL) $(topsrcdir)/toolkit/locales/compare-locales.pl $(srcdir)/en-US $(LOCALE_SRCDIR) > + $(call py_action,preprocessor,$(DEFINES) $(ACDEFINES) \ > + $(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/defines.inc \ > + $(LOCALE_SRCDIR)/defines.inc \ > + $(srcdir)/generic/install.rdf -o $(DIST)/xpi-stage/$(XPI_NAME)/install.rdf) Indeed that is mostly Bug 1210791, I'd rather we tackle it there since there is a lot of relevant discussion there. I'm happy to help review changes for this part in that bug. (r- for this reason)

>> Nit: it's not clear to me what the purpose of the second group here is. You don't seem to use it? Wouldn't: sed 's|^\([0-9]*\).*|\1|' work just as well? << I am not really a regexp expert. Just took it right out of the lightning makefiles. There it was for Thunderbird but I assumed that since TB uses the same version numbering scheme like Firefox it should be a 1:1 copy: calendar/lightning/Makefile.in Isn't this a separate patch and/or bug 1210791? Either way, this should be reviewed by someone who knows about the (SeaMonkey) build system. Added r?Callek for that. >> Isn't this a separate patch and/or Sorry I goofed and copied part of the complete patch. This shouldn't have been included here in the first place. And I even checked it twice before uploading...

This should be hopefully the right patch.

Comment on attachment 8716639 [details] [diff] [review] Version 2a patch which allows the standalone makexpi.py to work Review of attachment 8716639 [details] [diff] [review]: ----------------------------------------------------------------- This still has the hardcoded makefile fallback version numbers that Callek complained about. I don't think the Firefox case ever gets hit anyway, because seamonkey builds always have an up-to-date mozilla-central, and that would get accessed there, right? Besides seamonkey builds, the makefiles don't get used. So just getting rid of both of the hardcoded fallbacks seems sensible. With that, r=me

Review+ by Gijs Kruitbosch carried over from previous patch Additionally includes minor changes suggested by Callek to avoid redundancy. Tested with Seamonkey 2.42 de and 2.44a1 VS2015 x64 builds under Windows 7 Will set checkin-needed tomorrow if no one objects.

Gijs, do you know (or can you do) who does checkins for chatzilla ?

remote: https://hg.mozilla.org/chatzilla/rev/64823e8eb6d7