95944 - XPI packages not working on Linux 0.9.3

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect)

Description

[Andrea]

The XPI package installation on Linux seems to be broken on Mozilla 0.9.3 release.

How to test it:
  go on the URL specified in this bug and try to download locally for example
langitit.jar. Now open it inside Navigator. The usual dialog prompt the user if
he wants to install the package. If you proceed appears for just a sec. the
progress dialog and apparently the installation should finished.

I think anyhow Mozilla is not even able to open the zippy package, since no
files inside the package are not copied to the target dirs (yes, the write
permission are ok).

I've not the possibility to confirm if this is still true on a recent nightly.

Thank you all.

Comment 1

[Andrea]

To be more precise, this is a Linux only problem, since on Win and Mac seems to
be all fine.

The behaviour described above appears, not only when the XPI package is loaded
from locale, but also via network (try visiting the page
http://www.mozilla.org/projects/l10n/mlp_status.html and install a lang pack for
0.9.3)

Thanks again.

Comment 2

[Robert Kaiser]

When I tried my 0.9.3 langpack (de-AT), I first tried it on Linux (logged in as
root), and it did work well...

Comment 3

[Daniel Veditz [:dveditz]]

There's a dupe of this somewhere... no, those appear to be crashes.

Comment 4

[Andrea]

I guess if I have a bad understandig of the policy of Mozilla under linux.

It actually install correctly the lang pack when logged as root, but does not
prompt another user of the impossibility of lang packages as non-root user
(regardless the access rights the su gives to chrome/), not even this is cited
in the release notes (it's noted you need to install Moz under the home dir of
each user).

Which is the current policy Mozilla (the browser) is going to follow ?

In other words, is this a bug ?

Comment 5

[Daniel Veditz [:dveditz]]

This is a problem with the language pack install scripts, reassigning to Tao.
The script should probably detect the failure to install in the bin directory
and attempt to install in the profile directory. At the very least it should
print a clear error message into the install log, and then either return an
error code that the launching webpage could use to put up a message or put up an
alert itself.

Putting up an alert is problematic in the native install case, so we'd have to
work out a "silent" argument that could be used in that case.
I could've swore this was a known problem with a bug, but I can't find the duplicate

Comment 6

[tao]

I agree w/ dveditz that if 

 1. The user is downloading the pack from MLP page and
 2. the user is installing into a directory he does not have permission to, 

then, yes, the installer script should try the profile directory and print 
proper error message to the install log. However, if this problem happens 
even in installing from local disk to the user's profile directory. Then, we 
want to fix it in the installer engine .

Comment 7

[Andrea]

I'd like to point out the user is forbidden (in Moz093 linux at least) to
install the chrome/ and default/ directories even if these directories are write
accessible.

So the question is: is this a feature (= bug 95944 = INVALID), or is it a
software malfunction (we really have a valid bug).

The issues D.Verditz has rised are surely valid ones, anyhow.

Comment 8

[tao]

This seems to be a regression since we didn't have this problem in 6.1. I'l look
into this.

Comment 9

[tao]

>Additional Comments From [e-mail] 2001-08-24 12:10

>This is a problem with the language pack install scripts,

Are you referring to the "install.js" in langxxxx.xpi? If so, why are we not
seeing this problem in 0.9.2?

>The script should probably detect the failure to install in the bin directory
>and attempt to install in the profile directory.

I thought this is what we have been doing in xpinstall engine since N6.0. I 
believe the directory/folder permission problems (on Windows and Unix) were
explored and resolved in 6.0 timeframe.

Comment 10

[msanz]

adding nsbranch+

Comment 11

[tao]

The only way to reproduce this problem is to install the langpacks into a 
directory the user does not have write permission. In this case, it is not a 
regression, it is a known limitation.

I agree that Mozilla should prompt the user whether s/he wants to install the
packs into profile directory if s/he does not have write permission. This is in
fact a feature parity of langpack v.s. theme packs.

Reassign to xpinstall group. 

Comment 12

[tao]

didn't know that this bug will go to you.

Anyway, I recalled that in installing theme pack, a dialog prompt will pop up to
ask the user whether the theme pack will be installed into the global install
directory or the user profile directory.

We can probably apply the same approach here.

Comment 13

[Syd Logan]

Well, the italian and greek lang packs (the only ones I tried) seemed to
install, I got a dialog from the italian one at the end saying how to switch...
I went to Preference->Appearance and both greek and italian are now in the popup
menu. Neither shows (e.g., I still get English) after a restart, could that be
because of some incompatibility with 0.9.3 and 0.9.4?

all-locales.rdf got updated... but the URIs in there to me don't make much
sense. I'll attach the all-locales.rdf file, and an ls -l of the chrome
directory so it can be inspected to see if maybe the problem is where the files
got installed.

Comment 14

[Syd Logan]

removing sean

Comment 15

[Syd Logan]

Attachment: all-locales.rdf

Comment 16

[Syd Logan]

Attachment: chrome directory contents

Comment 17

[Syd Logan]

BTW, I have rw permission on the install directory. This is not a permissions
issue I would think.

Comment 18

[tao]

Hi, Syd:


To have a valid test, you need to run 0.9.3 build to install 0.9.3 langpacks. 
When I do so, the stated problem won't occur if I have write permission to 
the global install directory.

You are right that 0.9.3 packs are not compatible with 0.9.4 build.

Comment 19

[Syd Logan]

Tao, so, if you have write permission, this bug does not exist. And if you
don't? E.g., the original user to install is root, and then some other user,
say, "tao", logs in and tries to isntall a lang pack, this fails? Well, to me,
that seems reasonable, only root should be able to update the installation into
chrome.

Comment 20

[tao]

I think the purpose here is to keep users informed of what happens and let them
choose how to proceed next. In installing skin packs, the installer engine
prompts users where (global or profile directory) to install the skin pack.

IMO, we should do the same thing for langpacks.

Comment 21

[Daniel Veditz [:dveditz]]

There is no prompting in the theme installer, I have no idea what you think
you've seen there.  The theme installer always downloads into the profile and
never into the global location, to avoid just this problem.

You could write an install script which checks the return code of addDirectory()
and then tries again with getFolder("Profile") if that fails, or if you only
want to support locales installed into the global location then you could beef
up the langpack webpage with a trigger callback function that at least tells the
user the install didnt work.

Given users on Unix systems where they're not the administrator, I think the
first way would be better because it would allow them to install themepacks. The
other way just lets them know more positively that it failed so it's less
confusing but not otherwise helpful in letting them accomplish what they want.

Comment 22

[tao]

>There is no prompting in the theme installer, I have no idea what you think
>you've seen there.  

OK, I did a bit research. There was a dialog prompt in the theme installer.
However, it asks users whether to use the theme (right after installation)
instead of the installation location. Sorry, looks like I need some brain surgery.

But, could we tweak this prompt to ask user whether to install in the global
or profile directory? Is it safe to post such dialog from installer script?

Comment 23

[Syd Logan]

If the user is not root, we could put up a dialog saying that this change will
only affect the current profile, and if you want to make this locale available
to all users on this machine, you should restart the browser as root and re-install.

Another, perhaps better solution:

Make the browser setuserid so even if the user is not root we have permission to
do root things when we need to, like install stuff into chrome. This might be
considered a tad dangerous though by some, although we would just set the user
id to root for the time we need it and revert the user id to that of the user
who started the process immediately after we complete installing the files, so
the risk is minimized. 

Comment 24

[Syd Logan]

Oh, my other point is the best solution does not let the user know anything
about where we store these files, most grandmas aren't going to have a clue. But
we can say something like "You need to have the person who installed the browser
perform the installation, you do not have appropriate permissions". And relnote
this to let the administrator know what the issues are.

Comment 25

[Daniel Veditz [:dveditz]]

Attachment: patch to ns's windows/langenus.jst as an example

Comment 26

[Daniel Veditz [:dveditz]]

The patch was actually to the NS copy of the langenus.jst, but it looks the same
as the Mozilla one to me so it'll probably apply just fine. This simple script
change would solve the problem. The x.themes.org theme folks should do the same
thing, too, if they insist on using XPinstall instead of the chrome installer.

Comment 27

[tao]

Hacking the global install directory's permission, as you suggested, is probably
a bad idea. I like your first idea better:

"If the user is not root, we could put up a dialog saying that this change will
only affect the current profile, and if you want to make this locale available
to all users on this machine, you should restart the browser as root and
re-install."

However, unless we change the behavior of addDirectory() and registerChrome(),
this might not be easy to achieve from the installer engine.

I'm attaching a quick hack of a installer script for langenus.xpi.

Comment 28

[tao]

Attachment: a sample installer script

Comment 29

[tao]

interesting patch from dan :-)

1. I tried "LOCALE|PROFILE" as the 1st arg. of registerChrome and it didn't 
   run. "PROFILE_CHROME " is the right value.
2. Calling getFolder(pf, "Chrome") does not return the profile "chrome" 
   directory on Linux. It seems to be case sensitive.

Anyway, Dan's patch is more complete and cleaner. I suggest we take it for
langenus.jst.

As to the regus.jst, the US.jar portion can use the same approach. We can't
solve the problem for "defaults/", "plugins/", etc., though.

I suppose we just pop up an alert dialog to inform users of this situation.

Comment 30

[Daniel Veditz [:dveditz]]

> 2. Calling getFolder(pf, "Chrome") does not return the profile "chrome" 
>    directory on Linux. It seems to be case sensitive.

The first argument can be a key string (not case sensitive) or an existing
folder object. The second optional argument is a relative path--using '/' as the
delimiter on all platforms--and follows the host OS as far as name casing goes.
On Linux that's case sensitive.

Comment 31

[Jaime Rodriguez, Jr.]

Nominating for PDT+. This is a regression, and has been marked as nsbranch+

Comment 32

[tao]

Hi, Syd:


I am taking this back since I have a solution for langenus.xpi already. I am
still investigating the regus.xpi problem. Thanks for looking into this problem.

Comment 33

[tao]

Attachment: an install.js that failovers to profile directory when the shared directory is not writable

Comment 34

[tao]

Attachment: mozilla: failovers to profile directory after confirmation

Comment 35

[tao]

Attachment: ns: failovers to profile directory after confirmation

Comment 36

[tao]

I have come up with a patch based on dan's suggestion. I also added a dialog
to get confirmation from users before installing into profile directory.

Hi, Juraj & Dan:


Would you please review my patches?

thx

Comment 37

[Daniel Veditz [:dveditz]]

I'm uncomfortable with the alert, is it absolutely necessary? Why can't we just
silently fail over to something that works? The user asked for a language, and
got a language.

The rest looks good and I'll happily r= if you convince me on the confirm.

Comment 38

[tao]

>I'm uncomfortable with the alert, is it absolutely necessary? Why can't we just
>silently fail over to something that works? The user asked for a language, and
>got a language.
>
>The rest looks good and I'll happily r= if you convince me on the confirm.

My rationale was that normally the packs would be installed into the shared 
location. If this is not the case, we should inform the (power) users and let 
them make conscious decision. But, I guess most of them don't care anyway.

I'll take them out then. (I was glad to find that we do have this confirm() dlg
supported in xpinstall, though :-))

One quick question: 

I didn't submit patches for regus.jst since I am still investigating whether 
we can copy only "bin/chrome/US.jar" into the profile directory. The package, 
regus.xpi, contains some files that do not belong to the profile directory.

Can we say 

  addDirectory("","bin/chrome",fProgram + "/chrome","")? 

I am concerned about the cross-platform portability of this statement.

Comment 39

[Daniel Veditz [:dveditz]]

> Can we say 
>  addDirectory("","bin/chrome",fProgram + "/chrome","")? 

That won't work -- the third argument must be a folder object, and your
concatenation has coerced it into a string. You can replace it with
getFolder(fProgram, "chrome") though, if that's correct. You could also use the
4th argument to specify subdirectories, so
addDirectory('','bin/chrome',fProgram,'chrome') should do the same thing.

If this is like the langenus .xpi file, though, fProgram has already been
re-pointed at the chrome directory I thought.

Since regus.xpi has only one file in that directory, addFile() might be more
appropriate than addDirectory().
   addFile("","bin/chrome/US.jar",getFolder("Profile","chrome"),"");

In the addFile() case the 4th argument needs to be a full relative file name
(this allows files to be renamed), it can't just be the directory. The
equivalent to the above using the 4th arg would be
   addFile("","bin/chrome/US.jar",getFolder("Profile"),"chrome/US.jar")

Comment 40

[tao]

I am cleaning up the patches as suggested by dan (taking out confirmation dlg).

Also CC ssu for review.

Comment 41

[tao]

Attachment: moz: failover to profile folder when shared directory is not writable.

Comment 42

[tao]

Attachment: ns: failover to profile folder when shared directory is not writable.

Comment 43

[Sean Su]

yes, I agree.  No prompting is much better.

In both patches, I see:
 logComment("addDirectory() to " + fProgram + "failed! Prompting the user for 
action...");

when there's no more prompting code.  You should change that to reflect what is 
does now, which is fail over to the profile dir.

r=ssu once you get that logComment() changed.

Changing Platform and OS to All, since the patch is for all instead of just for 
linux.

Comment 44

[Sean Su]

Forgot to mention that we should test on all platforms since the patches are not 
just for linux.

Comment 45

[tao]

Attachment: moz: revised patches (for langenus.jst) based on ssu's suggestions about log comments.

Comment 46

[tao]

Attachment: ns: revised patches (for langenus.jst) based on ssu's suggestions about log comments

Comment 47

[tao]

The langenus.jst part is ready. I'll check them in as soon as tree is open.
 
I will submit the patches for regus.jst later.

Comment 48

[tao]

Attachment: (regus.jst) moz: failover to profile directory when the shared location is not writable

Comment 49

[tao]

Attachment: (regus.jst) ns: failover to profile directory when the shared location is not writable

Comment 50

[tao]

OK, I had attached the patches for regus.jst. The differences from langenus.jst
is that regus.xpi contain non-chrome directories. When the global location is
not writable, only "bin/chrome" is copied into profile/chrome.

One problem I found with addDirectory() is that it returns the error code of
the last directory copy. In other words, previous copy errors are ignored. This
is bad since the caller will not be able to know if one of copy (except last one)
fails.

In the submitted patches, I assume that the global directory's subdirectories
are either all writable or all unwritable. I believe this is a reasonable 
assumption.

Comment 51

[tao]

Attachment: repost::(regus.jst) moz: failover to profile directory when the shared location is not writable

Comment 52

[Daniel Veditz [:dveditz]]

AAAAH! addDirectory() has been coded wrong from the beginning! It's *supposed*
to return the first error it encounters. Now it could be quite happily returning
OK if the last file it adds happens to be fine.

No wonder corrupt archives gave us so many mysterious later errors --
addDirectory() wasn't catching the extraction error.
Damn damn damn. Filed bug 99670

Comment 53

[Daniel Veditz [:dveditz]]

Comment on attachment 49306 [details] [diff] [review]
(regus.jst) ns: failover to profile directory when the shared location is not writable

>Index: mac/regus.jst
>===================================================================
>+        err = addDirectory("","bin/chrome",fProgram,"chrome");

bin/chrome should be viewer/chrome in the mac version.

Comment 54

[Daniel Veditz [:dveditz]]

Comment on attachment 49338 [details] [diff] [review]
repost::(regus.jst) moz: failover to profile directory when the shared location is not writable

>Index: mac/regus.jst
>===================================================================
+        err = addDirectory("","bin/chrome",fProgram,"chrome");

Needs to be "viewer/chrome" for the mac version

Comment 55

[Asa Dotzler [:asa]]

0.9.4 is out the door.

Comment 56

[Asa Dotzler [:asa]]

0.9.4 is out the door.

Comment 57

[tao]

Attachment: moz: "bin/chrome" -> "viewer/chrome"

Comment 58

[tao]

Attachment: ns: "bin/chrome" -> "viewer/chrome"

Comment 59

[tao]

Diff of revised patches

tao=> diff 95944-regus-moz-7.pat 95944-regus-moz-6.pat
7c7
< +++ regus.jst 2001/09/17 21:54:26
---
> +++ regus.jst 2001/09/14 15:38:37
39c39
< +        err = addDirectory("","viewer/chrome",fProgram,"chrome");
---
> +        err = addDirectory("","bin/chrome",fProgram,"chrome");
67c67
< +++ regus.jst 2001/09/17 21:54:27
---
> +++ regus.jst 2001/09/14 15:38:37
127c127
< +++ regus.jst 2001/09/17 21:54:27
---
> +++ regus.jst 2001/09/14 15:38:37

tao=> diff 95944-regus-ns-7.pat 95944-regus-ns-6.pat
diff: 95944-regus-ns-6.pat: No such file or directory
[e-mail]-49=> diff 95944-regus-ns-7.pat 95944-regus-ns-5.pat
7c7
< +++ mac/regus.jst     2001/09/17 21:57:05
---
> +++ mac/regus.jst     2001/09/14 03:09:46
41c41
< +        err = addDirectory("","viewer/chrome",fProgram,"chrome");
---
> +        err = addDirectory("","bin/chrome",fProgram,"chrome");
70c70
< +++ unix/regus.jst    2001/09/17 21:57:05
---
> +++ unix/regus.jst    2001/09/14 03:09:47
132c132
< +++ windows/regus.jst 2001/09/17 21:57:05
---
> +++ windows/regus.jst 2001/09/14 03:09:47
                            

Comment 60

[Jaime Rodriguez, Jr.]

Need R/SR ASAP to be able to get PDT+.

Comment 61

[tao]

Attachment: ns: the corrected regus.jst patches

Comment 62

[Sean Su]

Comment on attachment 49657 [details] [diff] [review]
ns: "bin/chrome" -> "viewer/chrome"

wrong attachment

Comment 63

[Daniel Veditz [:dveditz]]

Comment on attachment 49704 [details] [diff] [review]
ns: the corrected regus.jst patches

sr=dveditz

Comment 64

[tao]

plan to check it tomorrow.

Comment 65

[Jaime Rodriguez, Jr.]

Tao - Have you gotten an R for this one???

Comment 66

[tao]

r=ssu,sr=dveditz

Comment 67

[tao]

into trunk.

Comment 68

[tao]

PDT:

This bug prevent language packs from being properly installed if users do not
have write permission to the global directory which is common in a network
environment. However, the langpacks and content packs are most useful to those
users who run the shared installation.

In addition, there is no alert or feedback prompt when the installation fails:
bad user experience.

Comment 69

[Jaime Rodriguez, Jr.]

The last patch need to have "has-review", and all other previous patches need to
be lined out. Other than that, pls come by @ noon for a chat and Express Approval.

Comment 70

[tao]

Comment on attachment 49283 [details] [diff] [review]
moz: revised patches (for langenus.jst) based on ssu's suggestions about log comments.

r=ssu based on ssu's 
comments on 2001-09-12 16:58

Comment 71

[tao]

Comment on attachment 49284 [details] [diff] [review]
ns: revised patches (for langenus.jst) based on ssu's suggestions about log comments

r=ssu, per 2001-09-12 16:58

Comment 72

[Daniel Veditz [:dveditz]]

Comment on attachment 49283 [details] [diff] [review]
moz: revised patches (for langenus.jst) based on ssu's suggestions about log comments.

sr=dveditz

Comment 73

[Daniel Veditz [:dveditz]]

Comment on attachment 49284 [details] [diff] [review]
ns: revised patches (for langenus.jst) based on ssu's suggestions about log comments

sr=dveditz

Comment 74

[Daniel Veditz [:dveditz]]

Comment on attachment 49656 [details] [diff] [review]
moz: "bin/chrome" -> "viewer/chrome"

sr=dveditz

Comment 75

[Jaime Rodriguez, Jr.]

Minusing this one. We should show a warning to a user telling them that it did
not install.

Jeff - Is this a thing a site administrator would want this control? Pls let us
know, if you want this for the next release.

Comment 76

[tao]

FYI:

1. Please note that the patches had been checked in into the 0.9.5 trunk. 
2. PDT's concern is that system administrator might not want users to download,
   install, or upgrade their existing language or reginal content packs.
3. I verified that this is NOT a regression. It is more of a feature parity 
   between skin and locale provider. Mozilla current always install skin packs 
   into the user's profile directory regardless whether s/he has the write 
   permission to the global chrome directory. However, Mozilla always installs 
   langpacks into the shared directory first. This proposed patch will failover
   to the user's profile directory when installation to the shared directory
   fails.

I am marking this fixed since it is in the trunk already. Please reopen if this
should be PDT+ or back out from the trunk.

Comment 77

[bobj]

> 2. PDT's concern is that system administrator might not want users to download,
>   install, or upgrade their existing language or reginal content packs.

PDT, Please reconsider.

Limiting the langpack XPI installers to only try to install in the Program
directory does not prevent users from adding langpacks to the profile.  Users
can simply copy the appropriate files into the profile directory. This just
makes it less convenient and less obvious.  If we really want to
give sysadmins this control, then there should be something like a lockable
pref which tells the browsers not to load themes, langpack, etc. from the
profiles.

1. The reason not to take the fix is based upon the theory that some users
   would not want this behavior.
2. This fixes real problems reported by real users.
3. The real users are probably the majority even if the theoretical users exist.
4. To address the theoretically undesired behavior, we should provide a
   mechanism which really prevents profiles from overriding global settings.

Comment 78

[bobj]

cc'd msanz

Comment 79

[msanz]

marking nsbranch+ for PDT to reconsider

Comment 80

[Jaime Rodriguez, Jr.]

Adding correctness Status Whiteboard, correct/expected behavior does not occur.

Comment 81

[msanz]

let's leave nsbranch+ nomination, we don't want to lose triage work

Comment 82

[Syd Logan]

nsbranch+ and PDT- are inconsistent. Can we leave nsbranch- without losing track
of the issue since nsbranch- bugs are certainly going to lead the list of
nominations in the next release?

Comment 83

[tao]

I am marking this as fixed since the patch is into the trunk already.

Comment 84

[msanz]

adding nsbranch+ again not to lose nomination value. Sorry for the extra mail... :-)

Comment 85

[msanz]

adding regression keyword, it was not broken pre-0.9.3

Comment 86

[msanz]

removed regression after talking to Tao

Comment 87

[lchiang]

teruko - can someone in IQA help to verify this bug?

Comment 88

[Teruko Kobayashi]

Changed QA contact to [e-mail].  Rui, please verify this.

Comment 89

[Rui Xu]

Verify fixed with mozilla 095 trunk build.

Comment 90

[Rui Xu]

Cannot repro it with mozilla 096 trunk build. Verified.

Comment 91

[Masaki Katakai]

Hi Tao-san and All,

I tried nightly build installed into shared directory and
Japanese Lang Pack from www.mozilla.gr.jp, but I always
got

  Installation failed. Error code was -239

on error dialog. Is this correct behavior? I'd like to
confirm what Mozilla should do here.

Comment 92

[Andrea]

Masaki Katakai: No, this is likely bug 109044.
This bug should only target the problems on installing while logged as normal 
user, with no write permission on the bin/chrome/ folder.