118140 - platformCommunicatorOverlay.dtd is missing in en-unix.jar/win32 and en-win.jar/unix

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Robert Kaiser]

0.9.7 and current builds miss platformCommunicatorOverlay.dtd in en-unix.jar on
win32 and in en-win.jar on Unix/Linux platforms.

That was a big problem for some L10n people doing 0.9.7 L10n work, as their
packs missing this file caused empty menu items for users on an other platform.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This was discussed at some point....  Ben was in on the scoop.

Comment 2

[Boris Zbarsky [:bzbarsky]]

In fact, see discussion and so forth in bug 113295

Comment 3

[rchen]

en-unix.jar is not used on win32 and en-win.jar is not used on Unix/Linux 
platforms. I don't see the problem. Please correct me if I am wrong.

I give this to Ben anyway.

Comment 4

[Robert Kaiser]

rchen, those files are heavily used by L10n people.

They are packed into the builds because of bug 72496 - L10n people shouldn't
have to download 3 browser packages just to get their XPI pack up and running...

Comment 5

[rchen]

Right. We should have one xpi for all platforms. It looks like build script 
missing something.

Comment 6

[Robert Kaiser]

This should be easy to fix, I think, as only the build scripts have to include
that existing file into the existing .jar pack.
I'm adding mozilla0.9.8 keyword because this already was a L10n blocker in 0.9.7
and caused lots of troubles, and it will block a bunch of people from doing
their L10n work if it'll be still in 0.9.8

Comment 7

[Hasbullah Pit]

suggestion:


how about if the nightlies build script pack a crossplatform langenus.xpi
for l10n ppl as reference. (currently we have to download the whole mozilla for
just langenus.xpi) . call it langenus-refer.xpi .

this might help the l10n ppl submit langpack on time.
the release also have langenus-refer.xpi .

Comment 8

[Robert Kaiser]

langenus.xpi should alsway be XP, see bug 72496 for details.

And yes, we have to download mozilla multiple times again now just for doing a
correct L10n XPI pack. CCing Tao, our L10n master, who fixed 72496...

Comment 9

[tao]

It's a valid bug. As Robert suggested, platform specific files should always be
packaged in the appropriate en-[platform].jar on ALL platforms. The installer 
script has the logic to choose which one to register.

Comment 10

[Robert Kaiser]

If somebody can tell me where the "magic" happens to get the files into the .jar
archives, I could look at it and provide a patch (I luckily have a trunk source
tree on my linux box)...

So, do we have anyone here who can help me?

Comment 11

[tao]

It's in (http://lxr.mozilla.org/mozilla/search?string=platformCommunicatorOverlay)

/xpfe/communicator/resources/locale/en-US/mac/jar.mn, line 3 --
locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd
/xpfe/communicator/resources/locale/en-US/os2/jar.mn, line 3 --
locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd
/xpfe/communicator/resources/locale/en-US/unix/jar.mn, line 3 --
locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd
/xpfe/communicator/resources/locale/en-US/win/jar.mn, line 3 --
locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd

I suspect build process does not pick it up.

Comment 12

[Robert Kaiser]

Toa, that's what I found, too - I just wondered where the build process is
informed to pick up those jar.mn files.
I believe the build process picks up
/xpfe/components/prefwindow/resources/locale/en-US/{unix|win}/jar.mn but not
/xpfe/communicator/resources/locale/en-US/{unix|win|...}/jar.mn
for the platform packages but I don't have a clue where the build process is
told to pick up one or the other jar.mn and that's where I stand now.
Does anyone know where to look for that?

Comment 13

[tao]

On Unix, look at Makefile.in (which generate Makefile at the same directory); on
Windows, makefile.win, and on Mac, look for something like this in
http://lxr.mozilla.org/mozilla/source/build/mac/build_scripts/MozillaBuildList.pm:

CreateJarFromManifest(":mozilla:extensions:irc:jar.mn", $chrome_dir, \%jars);

Comment 14

[Robert Kaiser]

Attachment: other than the comment about Mac, r=tao. r=sfraser for the mac stuff. Please test this patch on all three platforms before checking in.

I've now made a patch which I hope will resolve that problem. It adds the
directories/jar.mn files to MozillaBuildList.pm, Makefile.in and makefile.win
and adds makefile.win files to the unix and mac dirs, which only have
Makefile.in but no makefile.win currently.

I hope this one does apply correctly, I inserted the parts for adding the two
files manually - the rest is a "normal" cvs diff against the current tree.

I'm currently running my Linux build, and I hope it'll pick the files up but I
can't test on Windows and Mac - anyone here who can do that?

Comment 15

[Robert Kaiser]

My patched unix build did what I wanted it to do, I got the jars for win and mac
correctly now!


The patch seems to work correctly at least on linux ;-)

Can anyone try this on win and mac?

Comment 16

[Robert Kaiser]

There is an additional problem: messenger-mapi is also missing from en-win.jar
on Linux, at least.
It's excluded from the build process in
http://lxr.mozilla.org/seamonkey/source/mailnews/Makefile.in#31 and this looks
like the right thing... What should we do about his?
Tao, any idea?

Comment 17

[tao]

The linux and Mac makefiles should parallel the Windows'. We need to make sure 
the chrome url is correct: chrome://messenger-platform/locale/mapi/foo.*, though.

Comment 18

[Robert Kaiser]

could anyone review that patch and try to get this into the build, please?

Comment 19

[tao]

Comment on attachment 67073 [details] [diff] [review]
other than the comment
about Mac, r=tao. r=sfraser for the mac stuff. Please
test this patch on all 
three platforms before checking in.

please see my comments below.

>Index: xpfe/communicator/resources/locale/en-US/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/communicator/resources/locale/en-US/Makefile.in,v
>retrieving revision 1.13
>diff -u -d -b -r1.13 Makefile.in
>--- xpfe/communicator/resources/locale/en-US/Makefile.in	2002/01/24 15:10:37	1.13
>+++ xpfe/communicator/resources/locale/en-US/Makefile.in	2002/01/30 12:46:10
>@@ -26,15 +26,7 @@
> 
> include $(DEPTH)/config/autoconf.mk
> 
>-ifneq (,$(filter os2 windows,$(MOZ_WIDGET_TOOLKIT)))
>-DIRS 		= win
>-else
>-ifneq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT)))

You might want get review from sfraser about this part.

>-DIRS		= mac
>-else
>-DIRS 		= unix
>-endif
>-endif
>+DIRS		= mac unix win
> 
> include $(topsrcdir)/config/rules.mk
>

Comment 20

[Simon Fraser [no longer active]]

Be aware that 'mac' stuff in the makefiles applies *only* to the Macho OS X 
build, which is not the build that we ship. To make changes to the Mac CFM build 
(OS 9 and OS X), you need to edit the perl scripts in mozilla/build/mac/
build_scripts

Comment 21

[Robert Kaiser]

sfraser: the changes for mac build_scripts (MozillaBuildList.pm) are right at
the top of the patch file ;-)

Comment 22

[Dawn Endico]

simon, can you review the patch for mac? adding mozilla 1.0 keyword

Comment 23

[Simon Fraser [no longer active]]

r=sfraser for the mac stuff.

Comment 24

[Robert Kaiser]

Can anyone try that patch on Windows and Mac?
I'd also be happy to know that anyone else than me has tested it on Linux before
it can be checked in...

Comment 25

[Ben Bucksch (:BenB)]

I built with the patch on Linux. The last 2 (new) files seem to be checked in
already. The build went fine, the generated en-win.jar and en-mac.jar both
contain locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd.

Comment 26

[Stephen Walker]

The patch works fine for me on a gmake windows build. locale/en-US/communicator-
platform/platformCommunicatorOverlay.dtd was added to both en-mac.jar & en-
unix.jar.

Comment 27

[Marten ter Borgh]

Can someone please test this for mac?
This bug may seem trivial but it is _not_.
This gravely affects translating: to create a working build you have to fetch
three build (mac, unix, win), unzip the langenus.xpi, take the platform specific
jar for each platform from its respective build, and update your glossary
against those... far too much work.
I know of some translations that just don't work on platforms other than the
platform the translator(s) use- mozilla is supposed to be multiplatform, this
blocks that.
Please test this, pretty please fix this soon (we need this in mozilla 1.0, we
need it in  the RC too!)
I think the target should be set to mozilla 1.0 (it already has the mozilla1.0
keyword so that can't be a problem). Platform/OS should be All/All

Comment 28

[Robert Kaiser]

All/All is true for sure. I also hope we'll get this for 1.0, this should be
fixed to help L10n people - if we want to get real good L10n packs for 1.0

For post-1.0 I want to note that I've filed bug 133849 because the problem
underneath this looks much bigger for me - see my comments there...

Comment 29

[Robert Kaiser]

adding milestones, assigning to myself, as I've done the patch, and resetting
milestone.
This can't be just futured. I'll attach a second patch that does apply cleanly.

Should go in at least in 1.1 cycle and has a working patch, so I set a 1.1alpha
milestone as well.

Comment 30

[Robert Kaiser]

Attachment: Same patch, but does apply cleanly

Here is the new patch that does apply cleanly with "patch -p0 < patchfile". The
old one had problems with the makefile.win files.

Comment 31

[Robert Kaiser]

Comment on attachment 86081 [details] [diff] [review]
Same patch, but does apply cleanly

Transferring review from other patch, as this patch is exactly the same, only
that it applies cleanly.
Old comment said: "other than the comment about Mac, r=tao. r=sfraser for the
mac stuff. Please test this patch on all three platforms before checking in."

Could someone test this on the three platforms, please?

Comment 32

[dbragg]

Hi Robert,
I haven't tested it but your fix looks correct for that specific file problem. 
Unfortunately this problem is bigger than the xpfe/communicator directory. 
There are also files missing in the different platform jars from xpfe/global,
mailnews/mapi (missing from the en-win.jar on the mac and unix build systems)
and xpfe/components.  There's bug 144551 that I'm working on that fixes this bug
too. 

What I'm doing is taking all the platform-specific files and elevating them to
the jar.mn files in the various "root" directories.  This is not only cleaner
and easier to maintain but is necessary in things like mailnews/mapi which is
windows ONLY.

I hope to have a patch for 144551 today.

Comment 33

[timeless]

for the record making jar.mn's higher in the hierarchy really sucks.

it might be convenient for you, but it means developers who actually just want 
to build small chrome bits, are forced to rebuild much larger portions of the 
app.

Comment 34

[dbragg]

Ok. So I'll try not to move them to the "root" of particular area but having
specific jar.mn files in the platform directory is a maintenance headache as is
proven by this batch of bugs.  I will instead move the jar.mn files to the
common directory above the platform-specific dirs since I doubt that people are
going all the way down to an en-win directory to build some chrome. They can go
down to en-win's parent dir to build their chrome.

Comment 35

[Robert Kaiser]

I think it's quite bad this one missed the 1.0 train, as it's causing L10n
people some big headaches, and the patch for this one would have prevented lots
of propblems.

If a whole directory including its contents.rdf is missing from a pack, we see
some untranslated strings but nothing reall breaks. That's better than what
we're seeing here, though not ideal.

I filed bug 133849 some time ago for the bigger problem behind all that, and I
really think that recently mentioned bug 144551 is a dupe of that one...

Anyway, the most important thing is someone is working on this problem. Thanks,
dbragg!

Comment 36

[dbragg]

Yup, There's a few of these puppies lying around.  Basically I'll be making it
so that the en-win.jar created on Windows matches en-win.jar created on Mac and
Unix and vice-versa on the other platforms.  Hopefully my fix will make this
easier to maintain without bloating the build system.

There's definitely some dupes going on. The reason I'm using 144551 as the
"main" bug is that dveditz (who is the most knowledgable person in this whole
category) had commented in that bug on how to best fix it.

Robert, do you mind if I mark this one a dupe of 144551?

Comment 37

[Daniel Veditz [:dveditz]]

The patch is OK, but I'd rather see it fixed along the lines outlined at
http://bugzilla.mozilla.org/show_bug.cgi?id=144551#c22

That is, instead of making all build systems traverse platform-specific
directories, combine the jar.mn data into an existing jar.mn that's already
processed on all platforms.

Comment 38

[Robert Kaiser]

As long as the issue is getting fixed, I don't care what bug number it is worked
on (I only have to transfer the dependant bug 121010 to the other bug then)...
I think all bugs that are dupes of that issue should be marked as dupes to the
bug where the issue is really being worked on.

Additionally, we should also consider what to do with issues like I mentioned as
isuue 2 in bug 133849 Comment #2 - it's what is happening when I do a build
without e.g. LDAP: If the default build excludes one component that would
normally be included in e.g. en-US.jar, others that have this component will see
broken translation...

Comment 39

[[e-mail] (away - not reading bugmail)]

bug 144551 resolves this problem - I need someone to review the patch in bug 
144551 and have dveditz put his sr= stamp on it. Kairo, would you help?

You could also reassign this bug to me if you like, I can knock them both out at 
the same time. We need this ASAP.

Comment 40

[Robert Kaiser]

--> jbetak, see Comment #39

Comment 41

[Randell Jesup [:jesup] (needinfo me)]

This will not make 1.0.2; please nominate this and bug 144551 for 1.0.3 once
it's on the trunk.

Comment 42

[[e-mail] (away - not reading bugmail)]

closing this, will file a new bug for branch builds

Comment 43

[Teruko Kobayashi]

Changed QA contact to [e-mail].

Comment 44

[Kasumi]

Is it dup of bug 173552?

Comment 45

[[e-mail] (away - not reading bugmail)]

Kasumi, yes it's dupe of bug 173552 (branch) and bug 144551 (trunk).

Comment 46

[Kathleen :Brade]

verified