First patch. Deals with ./browser/locales and ./toolkit/locales

Comment on attachment 172386 [details] [diff] [review] Duplicate less logic I like it!

Oh. You left ./browser/locales/jar.mn with bad paths. We're not using ab-CD.jar!/locale/ab-CD since Fx 0.9

fixes paths to fx1.0 style

This patch fixes necko part (./netwerk/locales) to work with l10n.

this is necko part, with a fix for en-US builds

part 3 - fixing dom/locales to work with l10n.

wouldn't the changes in attachement 172433 make the localized stuff just hard to find? I don't think that tying the src directory structure to the one of the jar really brings a lot of benefit. At least, we should get module owner approval on this.

Comment on attachment 172451 [details] [diff] [review] Fixup client.mk [checked in] r=me, but we want to get all fixed different.

this patch clears ab-CD.jar a bit, and readds use of extra-jars.mn. Not tested on win yet

I have some meta code for doing checkout all locales the way I would want it. foreach (proj in MOZ_CO_PROJECTS) { foreach (loc in mozilla/$(proj)/all-locales) { foreach (foo in LOCALES_proj) {checkouts.push(l10n/$(loc)/$(foo))} } } checkouts.unique; This would mean that we declare the available locales for each project. It seems like a logical thing to do, and I find all-locales files to be a headache to maintain right now. The only problem is, I need to load all-locales files and I need to do uniq. I would thus prefer to code this in perl and call it. We allready use uniq.pl from modules.mk. Though only for BUILD_MODULES!=all, AFAICT. Adding the perl script to MOZCONFIG_MODULES would get it pulled on initial runs just fine.

Do you plan to create a toplevel mozilla/suite dir?

a) I plan to fail gracefully on not available all-locales. I don't think that we actually need to error there. b) If we port seamonkey to toolkit (bug 255807) with CVS l10n, I would expect to see a project tld that would contain an all-locales, yes.

Then I don't think you need perl: $(sort $(foreach project,$(MOZ_PROJECT_LIST), $(foreach locale,$(shell cat mozilla/$(project)/all-locales), $(foreach dir,$(LOCALES_$(project)),l10n/$(locale)/$(dir)) ) ) )

Comment on attachment 172451 [details] [diff] [review] Fixup client.mk [checked in] I checked this in, with the looping "all" structure in my comment.

Comment on attachment 172571 [details] [diff] [review] Fix paths in browser installer makefiles [checked in] --- Makefile:81: ../../../toolkit/locales/pl-PL/installer/windows/charset.mk: No such file or directory make: *** No rule to make target `../../../toolkit/locales/pl-PL/installer/windows/charset.mk'. Stop. --- I tested it on Linux so it might be not a problem on Windows. If so - r+ for me

What about pippki and pipnss?

patch to get mozapps/profile work with l10n. r=bsmedberg

paths to remaining issues from ./global: 1) http://lxr.mozilla.org/seamonkey/source/docshell/resources/locale/en-US/jar.mn 2) http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/jar.mn 3) http://lxr.mozilla.org/seamonkey/source/intl/strres/src/jar.mn 4) http://lxr.mozilla.org/seamonkey/source/embedding/components/ui/jar.mn 5) http://lxr.mozilla.org/seamonkey/source/embedding/components/webbrowserpersist/jar.mn 6) probably we're screwing something in http://lxr.mozilla.org/seamonkey/source/toolkit/locales/jar.mn because there are two profiles.properties lines 7) http://lxr.mozilla.org/seamonkey/source/browser/extensions/package-fixup/jar.mn 8) http://lxr.mozilla.org/seamonkey/source/gfx/src/jar.mn (http://lxr.mozilla.org/seamonkey/source/gfx/src/printdialog.properties duplicates /toolkit/locales/en-US/chrome/global/printdialog.properties)

Comment on attachment 172571 [details] [diff] [review] Fix paths in browser installer makefiles [checked in] Today's Firefox trunk installers from beast are absent. I did a quick survey and this is the only patch that landed that touched browser/installer/windows/.

Comment on attachment 172571 [details] [diff] [review] Fix paths in browser installer makefiles [checked in] >Index: browser/installer/windows/Makefile.in >=================================================================== >RCS file: /cvsroot/mozilla/browser/installer/windows/Makefile.in,v >retrieving revision 1.14 >diff -u -6 -r1.14 Makefile.in >--- browser/installer/windows/Makefile.in 6 Jan 2005 23:04:23 -0000 1.14 >+++ browser/installer/windows/Makefile.in 27 Jan 2005 16:44:06 -0000 ... >-include $(topsrcdir)/toolkit/locales/$(AB_CD)/installer/windows/charset.mk >+include $(topsrcdir)/config/rules.mk >+ >+include $(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/installer/windows/charset.mk ... > installer: ... >-include $(topsrcdir)/config/rules.mk I think by moving config/rules.mk above the installer target, this has changed the default target for this Makefile from installer to the Mozilla-wide default target. browser/installer/Makefile.in needs to be changed to invoke make in windows/ with the correct target (installer, in this case?) if this is the intended behaviour.

Got r=cmp over IRC for this fix.

Comment on attachment 172571 [details] [diff] [review] Fix paths in browser installer makefiles [checked in] >Index: browser/installer/windows/Makefile.in >=================================================================== >RCS file: /cvsroot/mozilla/browser/installer/windows/Makefile.in,v >retrieving revision 1.14 >diff -u -6 -r1.14 Makefile.in >--- browser/installer/windows/Makefile.in 6 Jan 2005 23:04:23 -0000 1.14 >+++ browser/installer/windows/Makefile.in 27 Jan 2005 16:44:06 -0000 ... > installer: > $(NSINSTALL) -D instgen >- $(INSTALL) $(addprefix $(topsrcdir)/toolkit/locales/$(AB_CD)/installer/windows/,$(LOCALIZED_FILES)) $(addprefix $(srcdir)/,$(INSTALLER_FILES)) instgen >+ $(INSTALL) $(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/installer/windows/,$(LOCALIZED_FILES)) $(addprefix $(srcdir)/,$(INSTALLER_FILES)) instgen This change leaves an extra parenthesis in this command. Builds now fail building the installer with: Syntax error: ")" unexpected make: *** [installer] Error 2 With a change that could (did!) break building the installers on Windows (something that's hung Tracy up in smoketests and required my time to diagnose in fixing), you should take greater care to test that your change actually works as expected prior to committing it. I'm tempted to ask you to back out the entire patch because it's obvious it wasn't ready for prime time but I'm not sure that's strictly necessary yet -- though I think a full patch review _is_ necessary as two bugs have snuck by already.

Comment on attachment 172571 [details] [diff] [review] Fix paths in browser installer makefiles [checked in] >Index: browser/installer/windows/Makefile.in >=================================================================== >RCS file: /cvsroot/mozilla/browser/installer/windows/Makefile.in,v >retrieving revision 1.14 >diff -u -6 -r1.14 Makefile.in >--- browser/installer/windows/Makefile.in 6 Jan 2005 23:04:23 -0000 1.14 >+++ browser/installer/windows/Makefile.in 27 Jan 2005 16:44:06 -0000 ... >+ $(INSTALL) $(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/installer/windows/,$(LOCALIZED_FILES)) $(addprefix $(srcdir)/,$(INSTALLER_FILES)) instgen I gather the above line should read: >+ $(INSTALL) $(addprefix $(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/installer/windows/,$(LOCALIZED_FILES)) $(addprefix $(srcdir)/,$(INSTALLER_FILES)) instgen

Comment on attachment 172571 [details] [diff] [review] Fix paths in browser installer makefiles [checked in] Committed a patch to fix the missing (IMO) addprefix call and kicking off another build on beast to double check that the fix holds.

note to myself: http://lxr.mozilla.org/seamonkey/source/accessible/src/base/jar.mn - I was sure it was already fixed.

> 1) http://lxr.mozilla.org/seamonkey/source/docshell/resources/locale/en-US/jar.mn > 2) http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/jar.mn > 3) http://lxr.mozilla.org/seamonkey/source/intl/strres/src/jar.mn dom/locales > 4) http://lxr.mozilla.org/seamonkey/source/embedding/components/ui/jar.mn toolkit/locales (needs #ifdef for seamonkey) > 5) http://lxr.mozilla.org/seamonkey/source/embedding/components/webbrowserpersist/jar.mn dom/locales > 7) http://lxr.mozilla.org/seamonkey/source/browser/extensions/package-fixup/jar.mn plugins.properties needs to live in dom/locales, and we can skip the communicator contents.rdf (this might require some hacking of the installer install.js scripts). > 8) http://lxr.mozilla.org/seamonkey/source/gfx/src/jar.mn > (http://lxr.mozilla.org/seamonkey/source/gfx/src/printdialog.properties > duplicates /toolkit/locales/en-US/chrome/global/printdialog.properties) This should be moved to dom/locales from both of the existing locations.

Updated patch. Removes duplicated locales

Benjamin: what about ./toolkit/locale ? Is it obsolate?

Ok. This patch cleans some of what is described in Comment #30: ./docshell/resources ./intl/uconv/src ./intl/strres/src

First attempt - ./mail/locales localizable

2nd part of locale/global clean up. Following Comment #30 this patch fixes points 4,5,7 and 8.

Comment on attachment 173697 [details] [diff] [review] 2nd part of locales/global cleanup [checked in] Fix the "no newline at end of file" and r=me

Issues left for Firefox: 1) accessible.properties are propagated to ab-plat.jar 2) http://lxr.mozilla.org/seamonkey/source/extensions/pref/autoconfig/resources/jar.mn 3) http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/jar.mn 4) http://lxr.mozilla.org/seamonkey/source/caps/src/jar.mn 5) http://lxr.mozilla.org/seamonkey/source/security/manager/ssl/resources/jar.mn and http://lxr.mozilla.org/seamonkey/source/extensions/webservices/security/src/jar.mn 6) http://lxr.mozilla.org/seamonkey/source/content/xml/document/resources/jar.mn 7) ./cookie 8) ./pippki 9) ./pipnss

(In reply to comment #37) > Issues left for Firefox: > 1) accessible.properties are propagated to ab-plat.jar dom/locales, but this will need a seamonkey #ifdef in dom/locales/jar.mn because the platform chrome is packaged differently. > 2) extensions/pref/autoconfig/resources/jar.mn toolkit/locales with an #ifdef > 3) http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/jar.mn dom/locales > 4) http://lxr.mozilla.org/seamonkey/source/caps/src/jar.mn dom/locales > 5.5 extensions/webservices/security/src/jar.mn dom/locales, but we need a= from jst/peterv/somebody (this is the same situation as xslt, which we already moved) > 6) content/xml/document/resources/jar.mn dom/locales > 7) ./cookie Ask dwitte: I think he had a bug to fix this hanging around already. > 5) security/manager/ssl/resources/jar.mn > 8) ./pippki > 9) ./pipnss Create a new security/manager/locales hierarchy for PSM.

Comment on attachment 173580 [details] [diff] [review] ./mail/locales >Index: mail/locales/jar.mn >=================================================================== <snip> > #ifdef XP_WIN >- locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (@AB_CD@/chrome/communicator-platform/win/platformCommunicatorOverlay.dtd) >+ locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (%chrome/communicator-platform/win/platformCommunicatorOverlay.dtd) > #else > #ifdef XP_OS2 >- locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (@AB_CD@/chrome/communicator-platform/win/platformCommunicatorOverlay.dtd) >+ locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (%chrome/communicator-platform/win/platformCommunicatorOverlay.dtd) > #else > #ifdef XP_MACOSX >- locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (@AB_CD@/chrome/communicator-platform/mac/platformCommunicatorOverlay.dtd) >+ locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (%chrome/communicator-platform/mac/platformCommunicatorOverlay.dtd) > #else >- locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (@AB_CD@/chrome/communicator-platform/unix/platformCommunicatorOverlay.dtd) >+ locale/@AB_CD@/communicator-platform/platformCommunicatorOverlay.dtd (%chrome/communicator-platform/unix/platformCommunicatorOverlay.dtd) > #endif > #endif > #endif Would it be possible to put these files in communicator-platform/win|mac|unix/ instead? A common en-US.jar for all platforms is handy for teams that use that file when translating.

We're moving to server-based system. You should use http://lxr.mozilla.org/l10n as example or mozilla/*/locales where *=dom|toolkit|netwerk|browser|mail.

Those platform ifdefs are awful; reminder to self to look up who added them and why. We do have platform-specific package capability builtin to the chrome reigstry which differentiates between win/unix/mac.

note to myself: They're conflicting on ./dom/locales/jar.mn :( It's because I made them independly. The jar.mn should be: locale/@AB_CD@/global/xslt/xslt.properties (%chrome/xslt/xslt.properties) locale/@AB_CD@/global/dom/dom.properties (%chrome/dom/dom.properties) locale/@AB_CD@/global/layout/HtmlForm.properties (%chrome/layout/HtmlForm.properties) locale/@AB_CD@/global/layout/MediaDocument.properties (%chrome/layout/MediaDocument.properties) locale/@AB_CD@/global/plugins.properties (%chrome/plugins.properties) locale/@AB_CD@/global/nsWebBrowserPersist.properties (%chrome/nsWebBrowserPersist.properties) locale/@AB_CD@/global/printdialog.properties (%chrome/printdialog.properties) locale/@AB_CD@/global/netError.dtd (%chrome/netError.dtd) locale/@AB_CD@/global/appstrings.properties (%chrome/appstrings.properties) locale/@AB_CD@/global/charsetTitles.properties (%chrome/charsetTitles.properties) locale/@AB_CD@/global/global-strres.properties (%chrome/global-strres.properties) locale/@AB_CD@/global/css.properties (%chrome/layout/css.properties) locale/@AB_CD@/global/xbl.properties (%chrome/layout/xbl.properties) locale/@AB_CD@/global/xul.properties (%chrome/layout/xul.properties) locale/@AB_CD@/global/printing.properties (%chrome/layout/printing.properties) locale/@AB_CD@/global/layout_errors.properties (%chrome/layout/layout_errors.properties)

updated patch so it uses automagic chrome:platformPackage="true"

+ <!-- Version Information. State that we work only with major version of this + package. --> This comment is incorrect, and should be removed.

Comment on attachment 173575 [details] [diff] [review] 1st part of locales/global cleanup [checked in] checked in.

Comment on attachment 173697 [details] [diff] [review] 2nd part of locales/global cleanup [checked in] checked in

Cleanup dom/dom.properties and layout/MediaDocument.properties so they go to locale/global

Comment on attachment 173568 [details] [diff] [review] ./mozapps/profile [checked in] r=bsmedberg, but fix the "no newline at end of file"

This caused bug 283046

Do these changes impact embedders (like Camino)?

Simon, they may affect embedders if you are using non-standard packaging techniques or are using the .properties files directly from your code. I have been doing LXR/mozilla queries to try and detect possible camino errors.

Comment on attachment 174918 [details] [diff] [review] move dom.properties, MediaDocument.properties to global [checked in] checked in

Comment on attachment 173568 [details] [diff] [review] ./mozapps/profile [checked in] checked in

So... Don't embedders use embed.jar? And isn't that packaged using embed-jar.mn (via gen-mn.pl and GenerateManifest.pm)? And wasn't that not modified by these checkins? So it looks to me like the set of changes that moved the Gecko locale files around without changing embed-jar.mn should have broken every single embedder, Camino included.

Comment on attachment 173979 [details] [diff] [review] ./mail/locales updated it's obsolte after Scott's checkins. I'll update patch asap

The patch that moved nsProgressDialog.dtd caused a regression in Thunderbird. We can no longer find the progress dialog DTD file, leaving XUL entity errors in the download attachment progresss dialog instead of actually showing the progress dialog. See Bug #284183 for more details.

So this patch made it so only NON XUL apps actually package up nsProgressDialog.dtd even though many of us use the helper app dialog and the associated download progress dialog. That seems wrong: http://lxr.mozilla.org/mozilla/source/embedding/components/ui/jar.mn#5 We still bundle the XUL files in the build, we just no longer bundle the associated DTD files.

For MOZ_XUL_APP, nsProgressDialog.dtd is supposed to be packaged from toolkit/locales/jar.mn. Gandalf, what happened here?

Ben regressed my patch with his checkin. Ugrrrrrrrrr! I can't understand the way he solved conflict here... http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespae_mode=show&file=jar.mn&branch=&root=/cvsroot&subdir=mozilla/toolkit/locales&command=DIFF_FRAMESET&rev1=1.11&rev2=1.12

Gandalf, I believe Ben's change was accidental, r=me to add them back in.

It looks like this checkin caused bug 284428 (busted XML prettyprinting in Firefox, because chrome://communicator/locale/xml/prettyprint.dtd loads nothing). Also, trunk firefox doesn't show the localized text for Expat errors. This is also a regression from this checkin, looks like; bug 283766 covers it.

In Thunderbird, I'm now getting the following XML error at startup: Couldn't convert chrome URL: chrome://communicator/locale/layout/xmlparser.prope rties followed by what looks like 30 or so NS_ENSURE_TRUE errors complaining about a string bundle: WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file c:/build/trees/prefs/mozi lla/intl/strres/src/nsStringBundle.cpp, line 250 WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file c:/build/trees/prefs/mozi lla/parser/htmlparser/src/nsExpatDriver.cpp, line 724 could be a similar issue to what Boris just brought up in the comment above this one.

Yes. I know about this issue. I'll try to send a patch today to finish ./communicator -> ./global migration which will fix those regressions.

This patch moves what left in ./communicator for Firefox. caps.properties security.properties prettyprint.dtd xmlparser.properties and accessible.properties Also, I think that embed-jars.mn require attention to clean it.

Comment on attachment 176577 [details] [diff] [review] rest of ./communicator stuff Requesting sr from peterv as Benjamin mentioned in Comment 38

Comment on attachment 176577 [details] [diff] [review] rest of ./communicator stuff >Index: dom/locales/jar.mn >+#ifndef MOZ_XUL_APP >+ en-win.jar: >+ locale/en-US/global-platform/accessible.properties Don't indent the "en-win.jar:"... it won't work. You need to include the () source locations also. >\ No newline at end of file Fix this! Finally, you may need to update http://lxr.mozilla.org/mozilla/source/mail/config/en-US-jar.mn

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

updating patch with Benjamin's comments. moa+ from doron

checked in

Comment on attachment 176577 [details] [diff] [review] rest of ./communicator stuff Sniff.

security part

Comment on attachment 177075 [details] [diff] [review] security part [checked in] I will fix newline while checking in

(In reply to comment #38) > > 2) extensions/pref/autoconfig/resources/jar.mn > > toolkit/locales with an #ifdef What #ifdef?

update after Scott's checkins. Benjamin: can I take your r+ from previous try, or you want to review it once more?

Comment on attachment 177095 [details] [diff] [review] maile/locales ttry 3 Did you test the communicator-platform bits? There are communicator files being shipped from xpfe/communicator also. >Index: mail/locales/Makefile.in >+DEPTH = ../.. >+topsrcdir = @top_srcdir@ >+srcdir = @srcdir@ >+VPATH = @srcdir@ >+relativesrcdir = mail/locales nit: fix alignment >Index: mail/locales/generic/chrome/messenger-smime/contents.rdf > <!-- Version Information. State that we work only with major version of this > package. --> >- <RDF:Description about="urn:mozilla:locale:en-US:messenger-smime" >-#expand chrome:localeVersion="__MOZILLA_LOCALE_VERSION__"/> >+ <RDF:Description about="urn:mozilla:locale:@AB_CD@:messenger-smime" >+ chrome:localeVersion="@MOZILLA_LOCALE_VERSION@"/> Just take this section out... localeVersion isn't used any more.

Comment on attachment 177075 [details] [diff] [review] security part [checked in] checked in

autoconfig bits

second try - forking contents.rdf

Something like this?

Comment on attachment 177392 [details] [diff] [review] Mail/SM editor patch Can we file a new bug for this thunderbird work? This bug is too long already ;)

bug 286108 filed. And we're ready with Firefox! Help is another issue.

Comment on attachment 172453 [details] [diff] [review] browser part I have confirmation from Pawell from Czech l10n that this patch works on Windows with both, cygwin and AS perl.

Comment on attachment 172453 [details] [diff] [review] browser part Please do not add the @AB_CD@... what's done is done, we don't need more churn. The makefile+#includesubst look ok, but please watch the tinderboxen carefully and be prepared to backout if there are problems.