54904 - Install scripts cannot select chrome

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Installer: XPInstall Engine, defect, P1)

Description

[Daniel Veditz [:dveditz]]

Install must be able to select chrome. without it when you install locales or 
skins they do not update the user-<chrome>.rdf files in the bin/chrome 
directory. This leads to a couple of major problems:

First, if you install an add-on component with a private skin then the 
component will not work -- period. The user will have to know how to edit the 
user-skins.rdf file manually. This is tempting certain developers to use the 
install to overwrite the existing user-skins.rdf file, which will certainly 
make their component work, but will break anything else registered there. Given 
more than one such component we would end up in an install-war.

Second, on the initial Mozilla/Netscape 6 install without a way to select skin 
and locale during the install then the user-<chrome>.rdf files will be updated 
as components are used, which on a locked-down system (e.g. Unix or corporate 
WinNT) cannot be done after the first run, leading to failure.

Comment 1

[Jimmy Lee]

Ccing David.

Comment 2

[David Hyatt]

You can force a selection using installed-chrome.txt.  Is that not good enough? 
 Also, the chrome registry will auto-select for you if nothing is selected, but 
it tries to do this only for the install dir (which is bad, since you may not 
have write access to the install dir).

Comment 3

[Pete Collins (MDG)]

I have a temp work around which works ok.
After xpi installs the package, i have an install.xul file which is a simple
dialog which calls "selectSkinForPackage" and does the right thing for the
package. 

The only problem is the user has to do this:

./mozilla -chrome chrome://my_package/content/install.xul

This is a lot to ask an end user to do . . .

--pete

Comment 4

[Pete Collins (MDG)]

>You can force a selection using installed-chrome.txt.  Is that not good enough?

How . . . ?

Comment 5

[Daniel Veditz [:dveditz]]

How do we know what to put in installed-chrome.txt? Updating the summary to 
indicate this is a deficiency in the install command set, not that we're 
lacking any chrome registry functionality.  We don't want add-on components to 
solve this problem by overwriting installed-chrome.txt themselves, that just 
leads to an install war.

Comment 6

[David Hyatt]

Yes, absolutely.  We should give people the capability to install and select 
right off the bat.  Make sure you give the capability to do the selection in the 
profile dir, since we won't want to be writing to the install dir.

Comment 7

[Pete Collins (MDG)]

Dan Dave. I have an idea that is a resonable solution i think.

How about for now making package authors responsable for including and
install.xul file. I have tested this method out all morning and it works great!

If there was a way to make a call to call "window.openDialog" at the end of the
install all of our problems will be solved.

I can call this dialog after the install happens the user presses the finish
button, the skin path is registered w/ selectSkinForPackage and the app then
launches. 

Clean and easy.

If you guys accept this method then we are already done.
You can test out what i am talking about here:

http://aphrodite.mozdev.org/installation.html

or here:

http://chameleon.mozdev.org/installation.html

It is actually pretty cool.

Dan, can we open a window from install.js w/out compromising "thread safety" ??

--pete

Comment 8

[David Hyatt]

Pete, since this dialog would presumably be the same for all packages, it seems 
like we should automatically display the appropriate dialog for selection on our 
own.

Comment 9

[Pete Collins (MDG)]

Well the only reason i was suggesting that the authors do it themselves is
because i have a real cool image of a chameleon as the background of my dialog.
It just looks *SO COOL* man!

;-)

--pete

Comment 10

[tao]

There is a discussion, "UI language switching", about selecting the initial
 default locale in an intranet deployment environment. Please read
news://news.mozilla.org/netscape.public.mozilla.i18n for details.

Comment 11

[Pete Collins (MDG)]

Attachment: Cool Dialog!

Comment 12

[Pete Collins (MDG)]

Dan, one note on install wars. I have been wiping installed-chrome.txt away with
the different packages i'm workin on and have not had any probs.

I tried *not* using installed-chrome.txt but it doesn't work w/out it . . .


It seems that this file is only needed once for initialization of the package.

Is this correct?

Thanks

--pete

Comment 13

[Daniel Veditz [:dveditz]]

The installed-chrome.txt is only needed once, but it's not processed until next 
start up. By "install wars" I meant that if you happened to install more than 
one package in the same session that used this technique they would stomp on 
each other, and the one installed first would not get registered/selected 
correctly.

Comment 14

[Phil Peterson]

PDT marking [rtm-]. It's not clear why Seamonkey needs this before we ship. If
skins work for our shipping components, we're done. Please renominate if we've
misunderstood.

Comment 15

[Daniel Veditz [:dveditz]]

Appealing PDT decision: they don't work, at least not well. 

With the select line put in installed-chrome.txt (which this fix would do) the 
right stuff gets written to user-skins.rdf at first start-up (and 
user-locales.rdf -- locales have the same problem). Without it nothing gets 
written until that component is used, which has been leaving mail and aim 
without a skin or locale if installed on a system that locks down the installed 
directory (as is common on unix, less common but not rare on windows NT).

The workaround would be to leave the install directory world writable, or 
hand-hack an installed-chrome.txt

Comment 16

[pete collins]

I need this for Netscape's Theme Builder FWIW. 

This certianly is a problem and quite frankly is half/assed the way it is
implemented right now.

--pete

Comment 17

[selmer (gone)]

Dan, this seems to point out a profile related problem.  We shouldn't have to
hack stuff into the install directory to handle user level choices.  It seems to
me that we're storing this info in the wrong place and shouldn't have
user-<anything>.rdf files anywhere in the install hierarchy.

Can you explain that part of it for me?

Comment 18

[Pete Collins (MDG)]

Dan, i have stumbled upon an issue w/ installed-chrome.txt.
On unix a carrage return is ^J and on windows it is ^M

It seems that the ^J's are barfing on windows.
How do i deal w/ this issue of platform control chars?

If i edit them out on windows it works fine.

Thanks

--pete

Comment 19

[Daniel Veditz [:dveditz]]

Steve: The two user-<foo>.rdf files in the global chrome directory are misnamed
IMO; a better choice would be default-skins.rdf and default-locales.rdf since
these contain the selections used when the user hasn't otherwise made a
selection recorded in the profile user-<foo>.rdf files.

Whatever the name, if you install on a unix system or locked-down NT system the
user-<foo>.rdf files generated on the first launch are incomplete.  This leads
to problems with SHIPPING CHROME, this is not some abstract nice to have. See
bug 53674 for one instance, there are similar problems with Mail and AIM.

Comment 20

[Phil Peterson]

PDT marking [rtm need info]. Please change to [rtm+] when fix has been code
reviewed.

Comment 21

[Kazu Yamamoto]

I'm working for Japanese language pack. 
Many end users said why langpack doesn't switch default global locale
to ja-JP in xpi installing.

If I add "locale,install,select,ja-JP" into installed-chrome.txt manually,
mozilla use ja-JP for default global locale.
But I don't know how to add this line into installed-chrome.txt automatically.

I'd like the option. Is there the method?

I can add the following line into installed-chrome.txt by 
writing "registerChrome(LOCALE | DELAYED_CHROME, getFolder(cf, chromeName), 
"locale/ja-JP/global/");" in install.js.

locale,install,url,jar:file://H|/mozilla/M18-10-6/bin/chrome/ja-JP.jar!/locale/
ja-JP/global/

Comment 22

[Daniel Veditz [:dveditz]]

That's exactly the problem this bug is aimed at solving.

Comment 23

[Daniel Veditz [:dveditz]]

Can't see that I'll get this by the PDT. Guess we'll have to ship a 6.01 upgrade
so l10n can install new locales correctly.

Comment 24

[Daniel Veditz [:dveditz]]

Moz 0.8 tasks 

Comment 25

[Daniel Veditz [:dveditz]]

No, we really do need to do this one because of theme upgrade problems, to make 
sure the user gets back to a safe state. Plussing.

Comment 26

[Jimmy Lee]

Nominating.

Comment 27

[Daniel Veditz [:dveditz]]

The raw chrome registry files have recently been collapsed into a single
chrome.rdf file for performance reasons which eliminates the workaround hack
localizers have been employing of overwriting one of the files to select the
default language. The same issues come up for default skins -- this is now
something we can't keep putting off.

Comment 28

[Marek Wawoczny]

Btw. You can still use chrome.rdf to setup default skin / language during installation.

Comment 29

[Daniel Veditz [:dveditz]]

Overwriting an existing chrome.rdf is evil -- you have no idea what packages,
skins or locales might be registered in there. It's an OK workaround for an
initial fresh installation from scratch -- is my redundancy making the point?
:-) -- but absolutely the wrong thing to do for an XPInstall to be doing.

Comment 30

[Marek Wawoczny]

It's working on fresh install and installed browser. Chrome.rdf is not overwritten when installing packages with chrome.rdf in properly installed browser (XPInstall method).

Comment 31

[Daniel Veditz [:dveditz]]

Taking these two

Comment 32

[Jaime Rodriguez, Jr.]

nsbeta1- per ADT/XPInstall triage.

Comment 33

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 34

[tao]

Hi, Dan: do you plan to work on this? If not, do you mind if we take it over? thx!

Comment 35

[Daniel Veditz [:dveditz]]

Feel free, I'd like to review though.

I was imagining a syntax like

  int status selectChrome( type, chromeName [, package] )

Where type is all the values registerChrome() supports except CONTENT (i.e.,
support PROFILE_CHROME and DELAYED_CHROME in addition to SKIN and LOCALE).
chromeName is self-explanatory, and the optional package is to allow authors to
select chrome for only those packages for which the chrome is provided. You
could support a list, or just make people enter multiple lines if you like,
probably a rare use. Not sure it's supportable in installed-chrome.txt
(delayed_chrome style).

Comment 36

[tao]

Hi, Juraj: Please work with dveditz on this. thx!

Comment 37

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

accepting

Comment 38

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

Dan,

just trying to establish what needs to be done. While surveying the source and 
useful APIs I stumbled upon this:

http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsJSInstall.cpp#1839
http://lxr.mozilla.org/seamonkey/source/rdf/chrome/src/nsChromeRegistry.cpp#1732

It seems like I need to flesh out the InstallSelectChrome function in 
nsJSInstall.cpp. On the chrome backend, I'm tempted to follow what's being done 
for SelectSkin and SelectLocale - namely using SetProvider.

I hope to be able to get this done by Sept. 4 - I'm also helping out Syd and 
Curt on MRE...

Comment 39

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

Arg, have to continue the "tradition" of pushing this out. Raising priority, I'd
really like to get this done soon.

Comment 40

[Syd Logan]

--> tao for reconsideration

Comment 41

[Rafael Ebron (:rebron)]

Installer triage team: nsbeta1-

Comment 42

[tao]

Hi, Bob: please reassign these i18n/l10n/l12y bugs accordingly. thx!