Closed
Bug 144551
Opened 22 years ago
Closed 22 years ago
inconsistent en-mac/en-unix in different platform langenus.xpi files
Categories
(SeaMonkey :: UI Design, defect, P1)
SeaMonkey
UI Design
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.2alpha
People
(Reporter: yinglinxia, Assigned: jbetak)
References
Details
(Keywords: intl, Whiteboard: has r= / needs sr=)
Attachments
(1 file, 8 obsolete files)
29.02 KB,
patch
|
netscape
:
review+
sfraser_bugs
:
superreview+
|
Details | Diff | Splinter Review |
Currently, the en-mac.jar and en-unix.jar are different between different platform langenus.xpi. For example, in win32 langenus.xpi, the en-mac.jar contains 7 files, but in mac langenus.xpi, the en-mac.jar contains 9 files. The xpi packaging needs to pick up those files from same place, to keep them consistent. Otherwise, the langenus.xpi won't be cross-platform.
Comment 1•22 years ago
|
||
which version? trunk? branch? I can confirm the problem on the trunk, at least, but don't know if we need to fix this for mozilla1.0 if it's on the branch. Please nominate as appropriate if this is expected to be fixed. Looks like bug 104485 was the culprit, over to Ben (note, this has nothing to do with the XPI packaging, it's the build system that creates the en-<plat>.jar files)
Assignee: dveditz → ben
Status: UNCONFIRMED → NEW
Component: Installer: XPI Packages → XP Apps
Ever confirmed: true
Reporter | ||
Comment 2•22 years ago
|
||
for all, trunk and branch
Comment 3•22 years ago
|
||
could you list the file name for each one. thanks
Reporter | ||
Comment 4•22 years ago
|
||
There are actually two problems in the en-mac/unix.jar. 1) In these 2 jar files, there are some inconsistent files between different platforms' langenus.xpi: - en-mac.jar: communicator-platform\*.* are in Mac xpi but not Win xpi; - en-unix.jar: platformCommunicatorOverlay.dtd is in Unix xpi, but not Win xpi. 2) In these 2 jar files, all file format (line breaks) are different between different platforms' langenus.xpi. The first problem could make the language pack (langXXXX.xpi) not really cross-platform. The second problem confused our localization tool, the software think all the files are total different. Ideally, all the en-win/mac/unix.jar should be exactly totally same in all platforms' langenus.xpi, file amount and file format.
I am lost here. Isn't the whole purpose of en-[platform].jar is to intelligently install only needed files on a specific platform? We are supposed to put win-specific files (such as winhook, n2p, etc) in en-win.jar but not the rest (en-mac.jar or en-unix.jar).
Reporter | ||
Comment 6•22 years ago
|
||
regardless installation, en-win/mac/unix.jar should be together in the langenus.xpi, which suppose to be cross-platform. For example, we always post Win32 langXXXX.xpi as language pack for all platforms.
Comment 8•22 years ago
|
||
yxia: again, please list the file names here.
>communicator-platform\*.*
what are *.* here?
please list them. so we can look at them.
Reporter | ||
Comment 9•22 years ago
|
||
communicator-platform\*.* means the directory itself and all files inside, contents.rdf and platformCommunicatorOverlay.dtd
Comment 10•22 years ago
|
||
> the directory itself and all files inside,
could you list all the file inside ?
Reporter | ||
Comment 11•22 years ago
|
||
2 files listed above
Comment 13•22 years ago
|
||
Nav triage team needs info: how does this affect end users? What is the effect of the missing files? Ben? Ying?
Whiteboard: [need info]
Reporter | ||
Comment 14•22 years ago
|
||
To end user: for example, if a Mac user download a language pack from our web, I think he will see some blank menu items: <!ENTITY closeCmd.label <!ENTITY printSetupCmd.label <!ENTITY quitApplicationCmd.label <!ENTITY redoCmd.label Because we always post win32 language pack on the web, and in that pack, the Mac platformCommunicatorOverlay.dtd is not in the en-mac.jar. I have not try it yet though...
Comment 15•22 years ago
|
||
No, missing DTD files to not result in blank strings, they result in blank windows because the XML doesn't validate. missing .properties files might result in the more benign blank strings. DTD's are way too fragile to have been a good choice for localization mechanism. Maybe we need to handle XUL in a Quirks mode that doesn't enforce the rule that all entities must exist :-)
Comment 16•22 years ago
|
||
yokoyama- could you please help to address the issue. thanks
Comment 17•22 years ago
|
||
what am I suppose to do here? I am kinda new in xpi arena. So by reading comments, we have to do: - add contents.rdf and platformCommunicatorOverlay.dtd into en-mac.jar and en-unix.jar - then update langenus.xpi Is this correct? If so, then do you really need my help? dan, can you own this bug?
Comment 18•22 years ago
|
||
This is not a xpi problem. The Xpi process is correctly slurping up exactly what the build process on each platform produces. The problem is that there are now some mac-only and linux-only jar.mn files so that the en-mac etc produced on the different platforms no longer match (ignoring line-ending differences, which don't matter to our parser). You need to make the windows and linux build process look like the mac process for en-mac. You need to make the windows and mac build process look like linux for en-unix, and you need to make the mac and linux build process look like windows for en-win. These all used to be the same, but a few new platform-only jar.mn files were introduced. These should probably get moved back up into the parent jar.mn
Comment 19•22 years ago
|
||
Who knows _how_ to do this? I certainly don't. Tao?
Comment 20•22 years ago
|
||
Hi, Don - would you please take a look? thx!
Comment 22•22 years ago
|
||
- Use lxr to search for the files in question - find which jar.mn they're in - work up the tree to find the common jar.mn they should be moved to - move the references to the new jar.mn, fixing up the paths as req'd - remove old single-platform jar.mn - remove platform build system references to those jar.mn
Comment 24•22 years ago
|
||
Hmm, isn't this more or les a dupe of bug 133849? I filed that one when I found out the inconsistency problem is much bigger than bug 118140 that is still one of the biggest headache for L10n people...
Comment 25•22 years ago
|
||
After discussion with dveditz I felt that the best place for the jar references for the platform-specific file was in the "root" jar.mn files. There's already a whole bunch of chrome-related files there so there won't be much of a problem for developers who just want to build chrome. They already had to go to this level anyway. The one exception to this is the winhooks. This is only one file so I added the directory to the unix makefile.in and a CreateFileFromJar entry in the MozillaBuildList.pm
Comment 26•22 years ago
|
||
Comment on attachment 86639 [details] [diff] [review] Patch for jar.mn and "make" file mods r=tao
Attachment #86639 -
Flags: review+
Comment 27•22 years ago
|
||
Comment on attachment 86639 [details] [diff] [review] Patch for jar.mn and "make" file mods Please do another diff with the -N option -- I want to see the obsolete jar.mn files and asssociated makefiles go away -- otherwise people can get suckered into updating the wrong files. Have you tested (or had build buddies test) on all platforms? >Index: mailnews/jar.mn >=================================================================== >+en-win.jar: >+ locale/en-US/messenger-mapi/pref-mailnewsOverlay.dtd (mapi/resources/locale/en-US/pref-mailnewsOverlay.dtd) >+ locale/en-US/messenger-mapi/mapi.properties (mapi/resources/locale/en-US/mapi.properties) >+ locale/en-US/messenger-mapi/contents.rdf (mapi/resources/locale/en-US/contents.rdf) I don't see these removed from anywhere else. Will these get processed twice on windows? At a quick glance looks like you could remove "locale" from DIRS in both makefiles in mozilla/mailnews/mapi/resources/, and then remove all the makefiles in locales and below. >Index: xpfe/communicator/jar.mn >=================================================================== >+ >+en-win.jar: >+ locale/en-US/communicator-platform/contents.rdf (win/contents-platform.rdf) >+ locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd (win/platformCommunicatorOverlay.dtd) >+ >+en-mac.jar: >+ locale/en-US/communicator-platform/contents.rdf (mac/contents-platform.rdf) >+ locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd (mac/platformCommunicatorOverlay.dtd) >+ >+en-unix.jar: >+ locale/en-US/communicator-platform/contents.rdf (unix/contents-platform.rdf) >+ locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd (unix/platformCommunicatorOverlay.dtd) These too, where do they come out of, and what makefile entries now need to be removed? There's a mozilla/xpfe/communicator/resources/locale/en-US/os2 directory, but according to the en-US Makefile.in the os2 folks use the win versions. Should we remove the os2 stuff to avoid confusion? Leave them but snip the makefile stuff up to the resources dir level (because you have to leave the content stuff platform-specific). >Index: xpfe/components/Makefile.in >=================================================================== >-DIRS = bookmarks directory download-manager filepicker find history search sidebar related regviewer xfer prefwindow shistory timebomb console autocomplete updates urlbarhistory intl resetPref killAll >+DIRS = bookmarks directory download-manager filepicker find history search sidebar related regviewer xfer prefwindow shistory timebomb console autocomplete updates urlbarhistory intl resetPref killAll winhooks > > ifeq ($(OS_ARCH),WINNT) >-DIRS += winhooks urlwidget alerts >+DIRS += urlwidget alerts > endif I don't see any mods to the winhooks Makefile.in, which up to now has assumed only win folks get there (thus my question above about platform build buddies). Wouldn't it be better to zap the winhooks/jar.mn and just move the lines up to mozilla/xpfe/components/jar.mn? >Index: xpfe/global/jar.mn Same thing, you've added stuff here but not removed the old mechanism. We'll eventually end up with the same problem again only harder to diagnose because it'll look like we've done the right thing. xpfe/browser appears to have the same problematic structure, although at a quick glance at least win and linux traverse through all three platform directories. I guess if that part's working we shouldn't mess with it on the branch, but I'd like a trunk patch to deal with it.
Attachment #86639 -
Attachment is obsolete: true
Comment 28•22 years ago
|
||
This diff shows the removed files.
Comment 29•22 years ago
|
||
Dan made a good suggestion about the xpfe/components/winhooks dir. It would be a can of worms to open up that tree to Unix. It's better to just move the one file that was listed in the jar.mn to the xpfe/components' jar.mn file. I didn't have that in the last diff. This also removes one modification to the Mac's MozillaBuildList.pm file. (a good thing)
Attachment #86890 -
Attachment is obsolete: true
Comment 30•22 years ago
|
||
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion r=tao
Attachment #86897 -
Flags: review+
Updated•22 years ago
|
Attachment #86897 -
Flags: superreview+
Comment 31•22 years ago
|
||
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion looks good
Comment 32•22 years ago
|
||
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion oops, sr=dveditz
Comment 34•22 years ago
|
||
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion The new chrome regsitry system has the side-effect to this patch; en-mac.jar becomes the provider on win/unix. We need to find a place to explicitly designate the chrome provider.
Attachment #86897 -
Flags: needs-work+
Comment 35•22 years ago
|
||
That problem is a build system artifact, the installed versions don't have that problem. Therefore it can be fixed by the build system. Figure out whether it's the first-registered or last registered that takes precedence (I'm guessing first). Order the jar.mn such that mac will get registered. Then in the windows and gmake scripts echo the appropriate registration line into installed-chrome.txt explicitly such that the correct platform is registered instead. Remember that the gmake system also handles win(and os2) and Mac os x so you'll have to ifdef which platform gets registered there.
Assignee | ||
Comment 38•22 years ago
|
||
tao, dan: if en-mac.jar is used as a chrome provider, it would show up in installed- chrome.txt, correct? I tried to verify that claim on my current gmake win32 build and two Mac builds (old-style and Fizilla OS X), but it doesn't seem to be the case. However, it seems like en-mac.jar is *still* out of sync. I'm seeing platformglobaloverlay.dtd in Mac builds - it's missing on win32 builds...
Status: NEW → ASSIGNED
Comment 39•22 years ago
|
||
Hi, Juraj: In a win32 or linux buid, we don't use en-mac.jar. Therefore, you shouldn't see it registered on non-Mac build. For classic OS9 build, you are right that en-mac.jar should be registered. Are you referring to a build with the patch http://bugzilla.mozilla.org/attachment.cgi?id=86897&action=view ?
Assignee | ||
Comment 40•22 years ago
|
||
Hi Tao, I just applied the patch, realizing that your comment might have been related to its effects, and not the current state of the build scripts :-)
Assignee | ||
Comment 41•22 years ago
|
||
dveditz: I've updated dbragg's patch and and opted to modify add-chrome.pl and make-jars.pl instead of "echoing" the correct platform jar into installed-chrome.txt to override previous entries. Do you think we could take such a change into the tree?
Attachment #86897 -
Attachment is obsolete: true
Comment 42•22 years ago
|
||
jbetak: I can't help with r= as you requested in bug 118140 Comment #39 because I don't know the original code good enough to really check what the patch does in detail. Sorry. Can't await having this all fixed though.
Assignee | ||
Comment 43•22 years ago
|
||
Verification builds look good on all platforms. Tao idicated we should run this by sfraser and seawood - cc'ing them for review. Could we get this into Moz 1.2a?
Comment 44•22 years ago
|
||
Comment on attachment 95168 [details] [diff] [review] updated patch, resolves the incorrect chrome provider issue raised by tao I'm not sure who the right person to review this behavior change is since warren left. My guess would be one of the xpapps guys. The OS detection really needs to be fixed though. It doesn't properly detect cygwin perl ($^O == cygwin) and linux isn't the only unix platform that we support. We should default to unix if win32 & macos are not detected.
Attachment #95168 -
Flags: needs-work+
Assignee | ||
Comment 45•22 years ago
|
||
Chris, thanks for having a look at this. I agree with the fix for other Unix platforms -- I haven't thought of that at all. In addition to this we could list the Unix jar files last in the corresponding jar.mn. This would prevent the wrong platform file from becoming chrome provider even if the detection failed. I'm not sure that I agree with the $^O == cygwin comment. I believe I tried that before and ran into some issues, the current Windows/cygwin detection seems to work quite well. I have to admit that I plagiarized Gerv's code, which means that there are at least two precedents in our code base: http://lxr.mozilla.org/seamonkey/source/config/make-chromelist.pl#71 http://lxr.mozilla.org/seamonkey/source/embedding/config/gen_mn.pl#15
Comment 46•22 years ago
|
||
Are you sure that you're using cygwin perl and that $win32 is being set to 1? ARCHANGEL:/cygdrive/c/root/mozilla> perl -e '$win32 = ($^O =~ /((MS)?win32)|os2/ i) ? 1 : 0; print "$win32\n"' 0 ARCHANGEL:/cygdrive/c/root/mozilla> perl -v This is perl, v5.6.1 built for cygwin-multi The other scripts that use this method of platform detection should be fixed as well (though not necessarily in this bug).
Assignee | ||
Comment 47•22 years ago
|
||
Chris, you are obviously right. I filed a new bug 166105 for cygwin perl support in make-chromelist.pl and gen_mn.pl. I still don't like the platform detection on Windows, what would you say to something less kludgy? my $win32 = ($^O =~ /win|os2/i) ? 1 : 0;
Attachment #95168 -
Attachment is obsolete: true
Comment 48•22 years ago
|
||
> my $win32 = ($^O =~ /win|os2/i) ? 1 : 0;
You can't do this - remember "darwin".
Gerv
Assignee | ||
Comment 49•22 years ago
|
||
oh, right :-)
Assignee | ||
Comment 50•22 years ago
|
||
Gerv, thanks for pointing out my oversight in regards to Darwin... I hope that this could finally be what we've been looking for. my $macos = ($^O =~ /MacOS|darwin/i) ? 1 : 0; my $win32 = (!$macos && ($^O =~ /win|os2/i)) ? 1 : 0; my $unix = !($win32 || $macos) ? 1 : 0;
Updated•22 years ago
|
Attachment #97479 -
Attachment is obsolete: true
Comment 51•22 years ago
|
||
Comment on attachment 97552 [details] [diff] [review] yet another patch iteration I preferred the previous win32 detection string though it still needed cygwin added to the list. Just grepping for /win/ seems to be asking for trouble.
Attachment #97552 -
Flags: needs-work+
Assignee | ||
Comment 52•22 years ago
|
||
thanks Chris - I'm changing back to my $win32 = ($^O =~ /((MS)?win32)|cygwin|os2/i) ? 1 : 0;
Assignee | ||
Updated•22 years ago
|
Attachment #97552 -
Attachment is obsolete: true
Comment 53•22 years ago
|
||
For Mac, these changes scare me. Make sure they are thoroughly tested for both MacPerl-based CFM builds, and Mach-O based builds, using both Classic and Modern skins. This seems like it could cause a bunch of Mac chrome regressions. Who runs add-chrome.pl and make-jars.pl for MacPerl builds? Is this a packaging-only thing? +my $macos = ($^O =~ /MacOS|darwin/i) ? 1 : 0; Does $^O work in MacPerl?
Comment 54•22 years ago
|
||
One more comment: > 2) In these 2 jar files, all file format (line breaks) are different between different platforms' langenus.xpi. ... > The second problem confused our localization tool, the software think all the files are total different. None of the changes in this bug address this issue, and it's really an issue with your localization tools. The tools _have_ to be smart about linebreaks.
Reporter | ||
Comment 55•22 years ago
|
||
Well, as long as the first cross-platform problem can be fixed, we can live with it then. Because for localization, we can just simply use the en-[platform].jar in the win32 langenus.xpi, for all other platforms - then we won't have line-break problem anymore. This means we will use the DOS-linebreak format files in our localized Mac builds, which won't be a problem - as long as the en-[platform].jar inconsistency problem can be fixed, we will be safe to use the win32 en-mac.jar in Mac build.
Assignee | ||
Comment 56•22 years ago
|
||
Simon: Mach-O-based builds work great. It appears that for the MacPerl-based build, we need to change Jar.pm http://lxr.mozilla.org/seamonkey/source/build/mac/build_scripts/Moz/Jar.pm#407 The only reason MacPerl builds worked for me is that the mac-specific files are listed last in installed-chrome.txt. This is due to the reordering of static jar manifest entries introduced by the rest of the patch. (Competing chrome providers are awarded precedence based on their position in the installed-chrome.txt files.) $^O works great with both MacPerl and on OS X. I do realize the risk of regressions and since the platform detection is "merely" aiming at elimination of unnecessary platform files in installed-chrome.txt, failure is not catastrophic. The most fragile platform (Unix? Mac?) could be simply listed last in that file by the virtue of reordering static jar manifest entries. locale,install,url,jar:resource:/chrome/en-unix.jar!/locale/en-US/navigator-platform/ locale,install,url,jar:resource:/chrome/en-win.jar!/locale/en-US/navigator-platform/ locale,install,url,jar:resource:/chrome/en-mac.jar!/locale/en-US/navigator-platform/
Assignee | ||
Comment 57•22 years ago
|
||
sfraser: this is an analogous change for Jar.pm, it excludes platform-dependent chrome files from being registered via installed-chrome.txt. In theory this minimizes the probability of a foreign platform file from becoming chrome provider.
Assignee | ||
Comment 58•22 years ago
|
||
Attachment #97613 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #97709 -
Attachment is obsolete: true
Assignee | ||
Comment 59•22 years ago
|
||
Would anyone dare to r=/sr=? Please? This has been tested to death and I can't see any issues in verification builds. The packaging change is on the top of the L10n and L12y wish list for 1.2. Let's get this in in time for the 1.2 release. Giving it a longer trunk baking time and QA exposure can only help to prevent any potential regressions. The original incarnation of this patch was reviewed by tao and dveditz. We had to broaden the scope due to some build system relicts pointed out by dveditz. Thanks Dan!
Assignee | ||
Updated•22 years ago
|
Whiteboard: [adt3] [ETA: 6/3/02] → have patch / need r= and sr=
Comment 60•22 years ago
|
||
Comment on attachment 100488 [details] [diff] [review] complete & updated patch r=cls
Attachment #100488 -
Flags: review+
Assignee | ||
Comment 61•22 years ago
|
||
thanks Chris! Simon would feel comfortable giving an sr=?
Whiteboard: have patch / need r= and sr= → has r= / needs sr=
Comment 62•22 years ago
|
||
not really. Maybe Dan V. can?
Assignee | ||
Comment 63•22 years ago
|
||
thanks Simon, I've sent him an email... I really hope this patch can make it into 1.2.
Assignee | ||
Comment 64•22 years ago
|
||
I hate to be nagging, but l10n really needs this. Dan has given sr= on the original patch and might be swamped right now. He is not responding to my emails. I'll try to track him down in time for the 1.2b freeze, but if anyone gave an sr= on the perl script changes, that would be great :-)
Comment 65•22 years ago
|
||
Comment on attachment 100488 [details] [diff] [review] complete & updated patch rs=sfraser
Attachment #100488 -
Flags: superreview+
Assignee | ||
Comment 66•22 years ago
|
||
thanks Simon!
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 67•22 years ago
|
||
*** Bug 173502 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 68•22 years ago
|
||
Yeah!!! With today's trunk, it looks great to me :-) MILLION THANKS to jbetak! This is going to benefit us a lot.
Assignee | ||
Comment 69•22 years ago
|
||
Ying, that's great! This was not a trivial change, and I'm still half expecting someone to slam me with some nasty regression. And I hope to hear from kairo and his experience with the change. I'm cc'ing dbragg for the bulk of the credit. I'd say that he and dveditz did most of the work. I only happened to be driving towards the end ;-)
Comment 70•22 years ago
|
||
I built today's CVS (after I re-pulled the files I had conflicts with, as I had 118140's patch applied in my tree), and all the files look fine on Linux here. I hope this really is resolved now. Thanks! Marking verified.
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 71•22 years ago
|
||
Thanks kairo! ying, rchen, mcarlson - does l10n need this on the branch (bug 173552)? If so, would early next week be a workable timeframe?
Comment 72•22 years ago
|
||
Thanks! early next week would be fine. This is so great!
Comment 73•22 years ago
|
||
nominating for the branch -- please seek drivers and adt approval to land.
Keywords: adt1.0.2,
mozilla1.0.2
Comment 74•22 years ago
|
||
Plussing for adt. Need checkin by cob 10/16. Please make sure to get all required approvals (drivers) before checkin if this is not already done.
Comment 75•22 years ago
|
||
*** Bug 173552 has been marked as a duplicate of this bug. ***
Comment 76•22 years ago
|
||
Has this landed on the branch yet?
Assignee | ||
Comment 77•22 years ago
|
||
Dan, yes it has - see bug 173552. Updating keywords.
Keywords: mozilla1.0.2 → verified1.0.2
Comment 78•22 years ago
|
||
*** Bug 172075 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•