Closed Bug 87622 Opened 23 years ago Closed 23 years ago

Linux n6.1: embed.jar located outside language pack in browser.xpi

Categories

(Core :: Internationalization: Localization, defect)

Product:
Core
Component:
Internationalization: Localization
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla0.9.7

People

(Reporter: rsmyth, Assigned: adamlock)

References

Details

(Keywords: l12y)

Attachments

(1 file)

Patch removes embed.jar from packages and renames embed.jar to embed-sample.jar. Reviews please
2.95 KB, patch
blizzard
: review+
dveditz
: superreview+
Details | Diff | Splinter Review 
N6.1 PR1 for Linux embed.jar contains localisable text and locale information 
and is located in browser.xpi.
This should be in the language pack.
Keywords: l12y
Status: UNCONFIRMED → NEW
Ever confirmed: true
Switching QA contact to jonrubin.
QA Contact: andreasb → jonrubin
Give it to amasri to follow up.
Assignee: rchen → amasri
Assigning to chak. All bare strings should be tokenized and placed under 
en-US.jar. All bare urls should be tokenized and placed under US.jar.
Assignee: amasri → chak
Keywords: nsBranch
mass change, switching qa contact from jonrubin to ruixu.
QA Contact: jonrubin → ruixu
Blocks: 99194
Msanz - This one looks untouched for a while. Any ideas? Shpoudl this one be
nsbranch- for this round?
embedding is not part of browser client. Do we care?
Agree with tao - embedding's not part of the browser client
Yes, we do care, because the public response to the client will help win our
embedding efforts.

So, let's see if we can help here. Why is this marked as nsbranch, if it has
nothing to do with the client??? Looks like L10N needs this to reduce the build
costs.

Propose we either minus this one for this round, or remove the nsbranch keyword.
The strings in embed.jar are mainly tooltips for the browser toolbar. There is
also locale specific information, (locale\en-US folder and contents.rdf) which
should not reside outside the language pack.
Right. This is not an Embedding bug, the file just happens to be called 
embed.jar

The strings in this file do appear in the commercial product
Why is embed.jar in there at all? AFAIK there is no reason to install that file
in the NS6.1 or Mozilla browser distributions.

I should note that embed.jar is used in two ways. First there is
mozilla/embedding/browser/chrome dir that builds an embed.jar that does have
some en-US locale stuff. Then there is the embedding distribution build that
appends files specified by the mozilla/embedding/config/embed-jar.mn manifest
file. The embed-jar.mn is locale neutral but defaults to en-US. See:

http://lxr.mozilla.org/seamonkey/source/embedding/config/embed-jar.mn

Any line that says XXXX will be replaced by whatever locale is specified when
gen_mn.pl is executed. The default is en-US but it could be anything else if
necessary.

So is this an installation issue?
Marking nsbranch- as it was decided in the August bug triage that we wouldn't
have enough time in eMojo to fix this.  Let's revisit for MachV.
Keywords: nsbranch-
removed keyword nsbranch since bug now has nsbranch-, per pdt mtg.
Keywords: nsbranch
The strings in embedding.dtd in embed.jar are duplicates of strings in
navigator.dtd (in the language pack), e.g. backButton.tooltip "Go back one page"
appears in both both dtds.
The problem is the Linux build uses the strings in embed.jar and not en-US.jar.
There doesn't appear to be any reason why these strings should be duplicated in
embed.jar. Surely the Linux build should point to the strings in navigator.dtd
in the language pack and embed.jar should be removed?
removing sscruton and adding laurasl
In theory, yes we could point our strings at Navigator's strings, but that adds
an undesirable dependency and pulls in a lot of strings we don't use. Embedding
is supposed to be a client of Gecko, not a client of Navigator.

I suggest the best course of action for now is to remove embed.jar from the .xpi
file and that localisers should ignore the chrome in mozilla/embedding. The
embedding chrome is just sample chrome anyway, meant as a reference for
embedders to write their own.
OK. Thanks Adam. 

Since this has no functionality we should probably remove this from the US 
Mozilla and Commercial build process for Mach V. 

In the meantime Owen can you modify our L10n scripts to exclude this jar from 
the localised Linux builds for eMojo.
->danielmc so it can be assigned to Owen per his comments above
[sorry, i do not know Owen's email address]
Assignee: chak → danielmc
IMHO, this is *NOT* a localisation bug, but a base bug which affects localisation.
Localisation builds should follow the same structure as Base(US) builds without
making any alterations except where there is a language specific
requirement(different translations etc.).  Therefore I believe that this bug
sould be fixed in the US builds rather than in any L10n builds alone.
I can remove this jar file from our localisation builds, *BUT* I am reluctant to
do so since this would create an inconsistancy between the US build and any L10N
builds.
As far as I can tell from the comments up to this stage this issue has nothing
specific to do with l10n'ing the US product.  This strings as mentioned should
be in a language pack.  This should be done in the US product and then
localisation should follow the lead, by making the same change.
reassigning to rchen. Ray - please reassign it to the core team as appropriate.
cc. yxia
Assignee: danielmc → rchen
Right. This jar should not be in the non-embedded product. Strings in this jar 
supersceed strings in the language pack (including en-US.jar).
Please clean up and eliminate embed.jar. embedding.dtd was created and editted 
by locka@iol.ie. Don't know whether he is still working on Mozilla. Reassigned 
to valeski to follow up.
Assignee: rchen → valeski
Blocks: 107067
Keywords: nsbranch-
-> adam
Assignee: valeski → adamlock
as Adam mentions embed.jar is presend in Mozilla as an example for other 
embedding projects so maybe there is some merit in leaving it in Mozilla. There 
is no good reason to keep it in the commercial product though.
Exactly embed.jar has a purpose as a sample file so I don't want to clean up or 
delete it. I can remove it from the Unix packages if you like.

I could also rename it to embed-sample.jar which would make it more 
obvious and also avoid some confusion with another embed.jar we produce for the 
embedding build.

Daniel' suggestion is good. Can we remove it in the commercial build?
I've looked in ns/xpinstall and embed.jar is not mentioned at all. That means
this patch should cover both commercial and non-commercial versions.

I don't believe there is a need for embed.jar (or embed-sample.jar as it is now)
to be in either version anyway. I'm CC'ing blizzard just in case there was a
legitimate reason he added it to the package list.
I think that I originally added it so that the little bits of XUL that I was
using for the gtk embedding widget would be in the browser package.  I don't use
them anymore so I suspect that it can be removed.

However, I would look and make sure that you aren't going to break anything by
removing it.
The contents of embed-sample.jar are this:

content/embed/contents.rdf
content/embed/mini-nav.js
content/embed/mini-nav.xul
content/embed/simple-shell.xul
content/embed/back.gif
content/embed/forward.gif
content/embed/reload.gif
content/embed/stop.gif
locale/en-US/embed/embedding.dtd
locale/en-US/embed/contents.rdf
content/embed/simple-shell.css
content/embed/embedding.css
skin/classic/embed/contents.rdf

There is nothing outside of embedding/browser/chrome that references these
files. In fact, Unix is the only platform that this chrome even gets built for.
You know what?  I'll bet we don't use any of those files at all.  Can we just
delete embed.jar entirely?
I saw no ill effects on ripping it out of 6.1 (but I did not do extensive 
testing...)
I would like to keep embed.jar renamed as embed-sample.jar but removing it from
the packaging. This is what the patch does.

It will become part of our "using chrome in embedding apps" story, serving as a
simple sample for embedders who want to write their own chrome. See bug 75538.
Can I have a review on this please?

I believe this patch is right and good though embed-sample.jar will probably
change further or disappear as our embedding SDK story takes shape.
Comment on attachment 56903 [details] [diff] [review]
Patch removes embed.jar from packages and renames embed.jar to embed-sample.jar. Reviews please

sr/r=blizzard
Attachment #56903 - Flags: review+
Comment on attachment 56903 [details] [diff] [review]
Patch removes embed.jar from packages and renames embed.jar to embed-sample.jar. Reviews please

sr=dveditz
Attachment #56903 - Flags: superreview+
Fix is in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla0.9.7
Verify fixed with build 2001-11-19 trunk on JA RH7.1.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: