328317 - merge platform .jar files into en-US.jar

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Robert Kaiser]

We should do what bug 102509 comment #25 proposes and Firefox/Thunderbird have done already: merging the platform .jar files into the main language .jar file.

Along with bug 325473, this is needed for the "source L10n" stuff of bug 286110, where we'll end up having a single @ab-CD@.jar in the end.

I'm currently working on a patch. The main stuff is easy, the trickier parts are the needed changes so that chrome registry gets the correct new locations set.

This mainly needs the langenus.jst file to register the new location (which is straight forward) but also installed-chrome.txt getting written correctly at build time, which is tricky and done by make-jars.pl mainly, see http://lxr.mozilla.org/mozilla/search?string=foreignPlatformFile

I guess I'll need to add an additional check for a "foreign platform path" instead of just a foreignPlatformFile (which we don't have any more) as we do now. That means we'll omit writing anything with |-platform/<foreignPlatform>/| in the path from writing to installed-chrome.txt (just as we do with |-<foreignPlatform>| in the file name at the moment).

Comment 1

[Robert Kaiser]

Attachment: temp patch, containing some stuff from bug 325473

This is a temporary patch that contains some stuff from bug 325473 (where it does changes in the same files). I'll do a cleaner one for full review once the other stuff has been checked in.

Comment 2

[Robert Kaiser]

Attachment: temp patch, containing some stuff from bug 325473

I missed some files in the first version of this. Here's the real temp patch...

Comment 3

[Robert Kaiser]

Comment on attachment 212939 [details] [diff] [review]
temp patch, containing some stuff from bug 325473

Benjamin, could you please have a look at the first two files changed in this patch (add-chrome.pl and make-jars.pl)?
We need those changes to still register the platform files correctly in instleed-chrome.txt at build time.
The rest of the changes will be reviewed later on...

Comment 4

[[e-mail]]

Note that in some cases we can simplify things, for example there's nothing to stop us from shipping nsWindowsHooks.properties on all platforms, or one platformCommunicatorOverlay.dtd can simply contain all the entities used by the separate platformCommunicatorOverlay.xul files.

Comment 5

[Robert Kaiser]

Attachment: better patch, non-cvs diff against tree with bug 325473

This patch does really only contain the work for this bug, but is no cvs diff but it's against a tree with the bug 325473 work, so only changes relevant to the platform stuff shows up in this one.

This is ready for review, but I'll probably replace it with a real cvs diff once the other bug's work has been checked in.

Neil, I don't fully understand what you mean... We're already shipping nsWindowsHooks.properties on all platforms, we're just only sticking it into the win-specific version of global-platform, which seems correct to me.
And separate .dtd files that look the same in en-US might be very useful, as esp. Mac sometimes has different usual localizations for the same English string than e.g. Windows.
I'd like to sort that stuff out in separate patches though, I'm currently only trying to get us to a situation where we have one single language .jar file, which we need for the source L10n approach - and here I'm only changing packaging of files while still leaving everything the same from the view of chrome (i.e. it's just chrome registry pointing the chrome paths to different on-disk locations)...

Comment 6

[Robert Kaiser]

Comment on attachment 213681 [details] [diff] [review]
better patch, non-cvs diff against tree with bug 325473

Again, bsmedberg, could you please review the changes to add-chrome.pl and make-jars.pl in this patch?

Comment 7

[Robert Kaiser]

Comment on attachment 213681 [details] [diff] [review]
better patch, non-cvs diff against tree with bug 325473

switching review for Build Config stuff over to Mark per IRC chat with him. Thanks for looking into that!

Comment 8

[Mark Mentovai]

Comment on attachment 213681 [details] [diff] [review]
better patch, non-cvs diff against tree with bug 325473

This change is fine when all you have to worry about is building for a single platform at a time.  I'm concerned about the implications it has on extension developers who are leveraging the Mozilla build system.  Developers may want to provide for multiple platforms in a single xpi by using chrome's PLATFORM_PACKAGE.

Comment 9

[Mark Mentovai]

Comment on attachment 213681 [details] [diff] [review]
better patch, non-cvs diff against tree with bug 325473

Ok, this is just a change for RegIt - that's fine.

Comment 10

[[e-mail]]

Comment on attachment 213681 [details] [diff] [review]
better patch, non-cvs diff against tree with bug 325473

>--- ../src/mozilla/xpfe/components/prefwindow/resources/locale/en-US/mac/jar.mn	2004-01-24 02:56:22.000000000 +0100
>+++ mozilla/xpfe/components/prefwindow/resources/locale/en-US/mac/jar.mn	2006-02-23 18:12:07.000000000 +0100
>@@ -1,3 +1,3 @@
>-en-mac.jar:
>-*       locale/en-US/communicator-platform/contents.rdf                  (contents-platform.rdf)
>-        locale/en-US/communicator-platform/pref/platformPrefOverlay.dtd  (platformPrefOverlay.dtd)
>+en-US.jar:
>+*       locale/en-US/communicator-platform/mac/contents.rdf                  (contents-platform.rdf)
>+        locale/en-US/communicator-platform/mac/pref/platformPrefOverlay.dtd  (platformPrefOverlay.dtd)
Unfortunately a (better) locale/en-US/communicator-platform/mac/contents.rdf                  was already included earlier in this patch (in communicator/jar.mn). You should kill these platform jar.mn files and move the platformPrefOverlay.dtd references to components/jar.mn instead. (Don't forget to fix up the makefiles as well.)

Comment 11

[Robert Kaiser]

(In reply to comment #10)
> Unfortunately a (better) locale/en-US/communicator-platform/mac/contents.rdf   
> was already included earlier in this patch (in
> communicator/jar.mn). You should kill these platform jar.mn files and move the
> platformPrefOverlay.dtd references to components/jar.mn instead. (Don't forget
> to fix up the makefiles as well.)

Sure, registering communicator-platform should be done from communicator/ - looks like we're currently already using that redundant contents.rdf, good catch ;-)

The other point is more difficult, as a simple move of the files to components/jar.mn would likely add those to XULRunner and Firefox, which isn't what we want. So either we add the ifdefs in jar.mn to match components/Makefile.in or we stay with the additional jar.mn files we currently have and only merge the stuff with the later patches for moving chrome over to suite/ - which of those approaches would you prefer?

Comment 12

[Robert Kaiser]

Oh, I've just seen that we don't build the whole components/jar.mn contents anyways for MOZ_XUL_APP, so I'll just move that stuff there. Look to me though that we should move all logic for prefwindow/ chrome there though, and remove all Makefiles in that dir...

Comment 13

[Robert Kaiser]

Attachment: moved prefwindow stuff (again non-cvs diff against tree with bug 325473)

This new patch fixes Neil's nits about prefwindow, and in that process it also merges the platformPrefOverlay.xul packaging into components/jar.mn, so that we can remove all Makefiles from prefwindow/ (which holds only chrome files).

Comment 14

[[e-mail]]

Comment on attachment 215664 [details] [diff] [review]
moved prefwindow stuff (again non-cvs diff against tree with bug 325473)

>+#  the mac file is unused!!!
I don't think this is appropriate here.

Comment 15

[Robert Kaiser]

Attachment: final path (real cvs diff of checked in version, for reference)

OK, here's the real cvs diff of the version I'm checking in right now, just for future reference. carrying forward reviews from previous attachments.

Comment 16

[Robert Kaiser]

This has been checked in now.