110919 - [rfe]Wrap Windows stub installer(s) in xpi for 6.x auto-install

Status: VERIFIED FIXED

(SeaMonkey :: Installer, enhancement, P2)

Description

[Curt Patrick (gone)]

Comment 1

[Curt Patrick (gone)]

Attachment: Creates mozillainst.xpi for mozilla

Comment 2

[Curt Patrick (gone)]

Attachment: Here's the jst file that goes with mozillainst.xpi

Comment 3

[Curt Patrick (gone)]

Attachment: Creates n6setup.xpi for ns tree

Comment 4

[Curt Patrick (gone)]

Attachment: Heres the jst file for n6setup.xpi

Comment 5

[Sean Su]

Curt, you might want to talk to marketing to see if this is 'exactly' what
they're looking for.  I ask this because at one point they wanted the installer
to be installed first, then executed.  This way, if the installer failed and/or
exited, they have it locally and could easily restart it.

Your current .jst files do not do this.  If the installer exits for any reason
before completion, the user will have to go look for the page that they found
the installer before, and redownload it.  Granted, the installer file itself is
small, but the effort of relocating it is probably an impedement for trying to
restart the installer.

Comment 6

[Curt Patrick (gone)]

Looks like you are right about that.  Where would be the best place to drop the
installer stub?  Somewhere in the temp folder?  Should we give user's the option
of changing that location?

Comment 7

[Sean Su]

It would probably be too cumbersome to provide the user with the option of where
to drop save the file (due to netscape.com page integration complexities).

I would recommend installing it to their desktop.  If under NT, I would install
under the current user only, unless someone thinks that the 'All users' folder
would be more appropriate.

Comment 8

[Gregg Landskov (gone)]

Yes I agree that the Desktop is the best place.

Comment 9

[Curt Patrick (gone)]

Attachment: install.js (for ns) that first installs to desktop

Here is a js file that should be pretty close.	Although it still launches
n6setup.exe file from w/in the xpi (actually, from the temp folder)which is
more straightforward, it first installs it to the desktop.

When we are satisfied with this install.js I'll cook an equivalent jst up for
both mozilla and ns.  But I think a couple things need considered before we're
ready for that:

If I cancel out the install the activity bar at the bottom right of the browser
continues to act like something isn't done.  Can I do anything about that?

Also, someone should take a good close look at the entries I'm feeding to
initInstall() and to addFile() to see if I'm feeding all the right values.  For
example, is it necassary to version either/both of these entries?  What about
the names I made up?  (Ditto for mozilla, if there are special considerations.)

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 64310 [details]
install.js (for ns) that first installs to desktop

>err = execute("N6Setup.exe", "", true);

You really don't want to set blocking to true ;-)

N6setup is going to ask the user to shut down the browser, which they
can't because the XPInstall process is waiting for N6Setup to finish.
Even worse, with recent changes it's not going to ask the user to shut
down until *after* all the stuff has downloaded, and it'll then wipe
out all the downloaded files as it shuts down and everything will have
to be re-downloaded.

We should really fix that in the installer. Currently we only save the
downloaded files if the download itself is interrupted. If the download
finishes OK then we always clean up. We should ask the user if they
want to save the downloaded files if they cancel.

Comment 11

[Curt Patrick (gone)]

Clearly I should have blocking turned off but, perhaps surprisingly, it doesn't
seem to have any effect on the behavior I'm seeing.  I tried both 

err = execute("N6Setup.exe");

and

err = execute("N6Setup.exe", "", false);

and the activity bar still remains active the entire time that the installer is
running and even after I cancel out.

BTW, even with blocking set to true I can still close the browser while the
installer is running.

Comment 12

[Sean Su]

Curt, did you check the task manager to make sure the browser was fully quit? 
Just a thought.

Also there's an easy fix that can be done to address Dan's concern about people
losing the downloaded files (after they have all been downloaded) if they decide
to quit the installer when asked to quit the browser instead.

The fix I propose is to move the code that unsets the flag indicating that the
download was successful until *after* the dialog asking the user to quit all the
browsers have returned successfully.  This way, it'll look as if the files did
not finished downloading.  There should be no harm in this.

Let me look for a quick patch.

Comment 13

[Sean Su]

I was just thinking that we probably don't want a "workaround", but rather the
right fix.  So I'll refrain from attaching the quick fix.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 63035 [details] [diff] [review]
Creates mozillainst.xpi for mozilla

There is code in dialogs.c like

   if(CheckInstances())
   {
      bSDUserCanceled = TRUE;
      CleanupXpcomFile();
      PostQuitMessage(0);

and the DeleteArchives() in extra.c only deletes if (!bSDUserCanceled).
So I guess the archives aren't blown away, but next start doesn't ask if
I want to reuse the archives. I guess we need to call
SetDownloadState() in there too for that to happen. Someone with a faster
network connection will have to play with this.

>+[mozillainst]
>+install\stub\mozilla-win32-stub-installer.exe

Planning ahead for this to be XP we should stick "win" in there
or else mozilla will never be able to have more than one platform
per directory, and noone will know which platform is in which file.

[mozwinstub] if you're worried about length, although you needn't
be: judging by the length of "mozilla-win32-stub-installer.exe"
mozilla.org values clarity above length.

There's nothing wrong with [mozilla-win32-stub-installer]

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 63037 [details] [diff] [review]
Creates n6setup.xpi for ns tree

sr=dveditz if you change it to N6Setup.xpi instead of lowercase n6setup.xpi --
URLs are case-sensitive and matching the .exe will cause less confusion.

Comment 16

[Daniel Veditz [:dveditz]]

Although none of the other .xpi files have mixed-case names... hm.

Gregg, your call.

Comment 17

[Daniel Veditz [:dveditz]]

instead of an install.js we'll need two .jst files with the appropriate
replacement variables for version, etc. in them, but basically it looks fine.

Comment 18

[Grace Bush]

this should be nominated

Comment 19

[Curt Patrick (gone)]

Attachment: Creates mozilla-win32-stub-installer.xpi

Comment 20

[Curt Patrick (gone)]

Attachment: mozilla-win32-stub-installer.jst

Comment 21

[Curt Patrick (gone)]

Attachment: n6setup.jst

Comment 22

[Daniel Veditz [:dveditz]]

Now you're no longer installing the file to the desktop, is this right?

Btw you should "cvs add" your new files before generating patches, then you can
use the "cvs diff -N" to include them in the same patch as your other changes.
Much goodness: reduces attachment clutter and makes it easier to review or apply
the patches, and by forcing the "cvs add" it's less likely you'll forget to
check in the new file which is a constant source of tree bustage.

Still waiting to hear from Gregg about N6Setup.xpi vs n6setup.xpi -- shall I
reassign it?

Comment 23

[Sean Su]

If the last two attached .jst files were a mistake, I have a comment on the .js
file that does install the stub file to the desktop.

I would suggest to use File.execute() on the file installed to the desktop
instead of on the one inside the .jar file.  I'm afraid that it might keep the
browser running when the installer asks them to quit it, since it can't delete a
running process.  Executing the copy installed on the desktop (as a non-blocking
process) should get around all of this.

Comment 24

[Daniel Veditz [:dveditz]]

We don't want to File.execute() because then we have to have two
initInstall/performInstall() sections, one to add it, and then one to execute
the file that is there. (File.execute() will return an error if you try to
execute a file that isn't there, even if you're sure it'll show up by time the
queued execute command gets run.)

There shouldn't be any blocking issues. Obviously blocking wasn't quite right,
but it doesn't matter in this case because the N6Setup.exe wrapper appears to
launch setup.exe non-blocking and then exit anyway. And cleanup isn't an issue
since if we can't delete the running process we simply mark it for deletion next
time.

Comment 25

[Sean Su]

actually, N6Setup.exe waits for setup.exe to finish because it has to clean it
up afterwards.  So, I'm not exactly sure what's going on with the blocking issue
being a non-issue. 

Comment 26

[Gregg Landskov (gone)]

For the name, I would prefer N6Setup.xpi (uppercase).  The only reason is for
consistency sake though.

Comment 27

[Gregg Landskov (gone)]

To see a really good example of how it should work...Currently from
Netscape.com, we are serving Winamp as an .xpi for 6.x users, and as a .jar for
4.x users.  

The download auto-runs, installs, and puts the installer file on the desktop. 
In the instance of the .jar file, the "high risk" comes up, but just ignore that
for now.  Also, it doesn't need to shut down any windows apps to install either,
but I thought it might be nice to see the examples.

Details to demo:
Netscape.com for 4.x -  
http://music.netscape.com/music/winamp/winampsellj.html which points to:
ftp://ftp.netscape.com/pub/communicator/smartupdate/english/windows/windows95ornt/winamp/winamp27ns.jar

Netscape.com for 6.x -  
http://music.netscape.com/music/winamp/winampsellx.html which points to:
ftp://ftp.netscape.com/pub/netscape6/smartupdate/english/windows/win32/winamp/winamp27ns.xpi

Comment 28

[Curt Patrick (gone)]

Attachment: Everything that ns needs

For reasons that I'll not go into, I had the dickens creating the .jst with all
the right stuff in it.	(Greenhorn)  This should install the installer to the
desktop, and creates N6Setup.xpi (mixed case).	I'm still going with Dan's
suggestion to launch the installer from the xpi until someone proves me wrong. 
I've tried it and it seems to work just fine.

Comment 29

[Sean Su]

Comment on attachment 65134 [details] [diff] [review]
Creates mozilla-win32-stub-installer.xpi

looks goood.  r=ssu

don't forget to attach the correct .jst file for the mozilla tree.

Comment 30

[Sean Su]

Comment on attachment 65207 [details] [diff] [review]
Everything that ns needs

running the copy from the .xpi file (instead of the one on the desktop) is the
correct way right now.

r=ssu

Comment 31

[Curt Patrick (gone)]

Attachment: Here is mozilla, too.

Comment 32

[Sean Su]

Comment on attachment 65209 [details] [diff] [review]
Here is mozilla, too.

r=ssu

Comment 33

[Daniel Veditz [:dveditz]]

Comment on attachment 65207 [details] [diff] [review]
Everything that ns needs

>+
>+[N6Setup]
>+install\stub\N6Setup.exe,N6Setup.exe

Do we know that this really works? Doesn't the package copying step happen
before .xpi files are created, while N6Setup.exe contains a processed
config.ini and other files that are created later. I think you're 
packaging up the previous build's executable!

For a netscape final release this isn't going to matter since the
contained info stops changing, but for mozilla and daily commercial
builds one day's stub will pull from the previous day's directory.

I think you're going to have to put the copy in makeall.pl, and if
the executable isn't created until after the makexpi step you might
have to do this as a one-off late in the script rather than as part
of the normal components.

>===================================================================
>RCS file: n6setup.jst

cvs is case sensitive, before checking this in please cvs rm this
version and cvs add it as N6Setup.jst to match the end result

>+err    = initInstall("Netscape Installer", "/Netscape/Netscape Installer", "$Version$"); 

Maybe "Netscape 6 Installer" instead of just "Netscape Installer" in
both arguments.

>+  err =  addFile("Netscape installer",

leave this first arg the default ("") and it'll pick up the
filename, which makes more sense than creating the key
"/Netscape/Netscape [6] Installer/Netscape [6] Installer"

Comment 34

[Curt Patrick (gone)]

Dan, you are right about my non-fix in packages-win.  I think I've got a handle
on how to make that happen in makeall.pl and will get on that today.

Comment 35

[Curt Patrick (gone)]

Attachment: Updated for mozilla

This builds the xpi in makefile.pl (instead of making changes in packages-win.
No change to the jst file.

Comment 36

[Curt Patrick (gone)]

Attachment: Update for ns

Comment 37

[Sean Su]

Comment on attachment 65633 [details] [diff] [review]
Updated for mozilla

change:
 +if(system("copy $inDistPath\\stub\\$seiFileNameSpecificStub
$inStagePath\\$seiStubRootName"))
 +{
 +  die "\n Error: copy $inDistPath\\stub\\$seiFileNameSpecificStub
$inStagePath\\$seiStubRootName\n";
 +}

to use $gLocalTmpStage for consistency, like so:

 +if(system("copy $inDistPath\\stub\\$seiFileNameSpecificStub
$gLocalTmpStage\\$seiStubRootName"))
 +{
 +  die "\n Error: copy $inDistPath\\stub\\$seiFileNameSpecificStub
$gLocalTmpStage\\$seiStubRootName\n";
 +}

r=ssu if you change that before you check it in.

Comment 38

[Sean Su]

Comment on attachment 65642 [details] [diff] [review]
Update for ns

same here.  see comments on the patch to the mozilla tree.

r=ssu if you also make the same change.

Comment 39

[Daniel Veditz [:dveditz]]

Comment on attachment 65633 [details] [diff] [review]
Updated for mozilla

>+# create the xpi for launching the stub installer
>+print "\n**********************************\n";
>+print "*                                  *\n";
>+print "*  creating stub installer xpi...  *\n";
>+print "*                                  *\n";
>+print "************************************\n";
>+if(-d "$inStagePath\\$seiStubRootName")
>+{
>+  unlink <$inStagePath\\$seiStubRootName\\*>;
>+}

I don't think we're supposed to be unlinking directories, but maybe
we get away with it because this is a windows-only script. Anyway
you're just doing what's already here so I'm not asking you to change.


>+err    = initInstall("Mozilla Installer", "/Mozilla/Mozilla Installer", "$Version$"); 

In the second arg the "author/organization" name is "mozilla.org"
(note all lowercase, too), "Mozilla" is the product. Please change to

"/mozilla.org/Mozilla Installer"

with that minor change sr=dveditz

Comment 40

[Daniel Veditz [:dveditz]]

Comment on attachment 65642 [details] [diff] [review]
Update for ns

>+err    = initInstall("Netscape 6 Installer", "/Netscape/Netscape 6 Installer", "$Version$"); 

This one is fine.

sr=dveditz

Comment 41

[Curt Patrick (gone)]

Patches are checked.

Comment 42

[Grace Bush]

Curt,

When I tried running this today (1/29), I double clicked on the N6Setup.xpi file
-which we should probably advertise as to what it does and where it can be
located-this gave me option to save to disk or open
I chose open and winzip came up showing me install.js and N6Setup.exe.  I
clicked on the exe file and installer started up.

what if the user does not have winzip?  I expected that at first click on the
xpi file that I would launch the Installer- it being saved behind the scenes. 
Am I incorrect in that? 

Comment 43

[Curt Patrick (gone)]

As I understand it, almost all customers will come into contact with this xpi
via a trigger in a page somewhere that Gregg will coordinate, not by double
clicking on it. So that is what we probably should be testing.  Double-clicking
will work only if the xpi extension is associated with the Netscape browser.  I,
like yourself, have xpi extensions associated with WinZip on my machine for
development purposes.  If I want to emulate the customer experience I drag and
drop the xpi onto my browser.  In general, though, only user who know where to
look and, by inference, know what a xpi is about, will be likely to try to
launch the xpi directly.  So, I'm back to saying that the only thing that seems
to me to count is to test this xpi from a trigger page.

Gregg, wouldn't it be a good time to mock up a page like what we are going to
provide at release time and let QA whack at the page and the xpi simultaneously?
 Who does that kind of thing?  I could put together a test page but I'm thinking
we need to get started putting the real-ish thing together somewhere anyway?

Comment 44

[Curt Patrick (gone)]

Grace, when you double click on the xpi, are you doing so from within the
browser or from the file system?  Double clicking it via the browser (i.e., ftp
or http) should launch it correctly anyway.  I was thinking of double clicking
on it in Explorer or some equivalent, in which it makes sense for WinZip to be
called.

Comment 45

[Daniel Veditz [:dveditz]]

Would it be possible to get this one .xpi file delivered up a level, in the same
directory as N6Setup.exe and n6setup.jar ?

Comment 46

[Gregg Landskov (gone)]

Yes, that is where I was planning on requesting that it be pushed.

Comment 47

[Grace Bush]

per mail that only users with N6.x will be able to use this auto installer- I am 
verifying on build for 2002012903

Comment 48

[Curt Patrick (gone)]

Dan, moving this xpi up a level is a very straightforward one line fix in
makeall.pl:

< if(system("perl makexpi.pl $seiStubRootName $gLocalTmpStage $inDistPath\\xpi"))
---
> if(system("perl makexpi.pl $seiStubRootName $gLocalTmpStage $inDistPath"))

This bug is closed and verified.  Do I need to open another bug for this, reopen
this bug, just get r= and sr= and check it in? or what?

Comment 49

[Jon Granrose]

So we're delivering a stub installer wrapped inside an xpi file now?  And the
current plan is to go completely away from the n6setup.jar file?  In that case,
we we stop building and delivering the n6setup.jar file, right?

I'm seeing the N6setup.xpi file now so I'm assuming this is checked in already.
 If you change the script to put the file above the xpi directory, we'll need a
bug filed against leaf to modify the win32 automation to deliver the file to
sweetlou since files outside the xpi directory are delivered on a file by file
basis.

Comment 50

[Gregg Landskov (gone)]

Yes we will be delivering N6Setup as an .xpi file at the same level as the .jar
and .exe.

But no, we don't want to stop delivering the .jar file.  We still want to serve
it to certain 4.x users.

The .exe, .jar, & .xpi should all be at the same level.

Comment 51

[Curt Patrick (gone)]

I didn't mention that I intend to provide the xpi for both mozilla and ns.  Leaf
tells me that he can handle delivering the ns version but someone in mozilla
land needs to decide if they want to pick up the mozilla version of the xpi and
deliver it accordingly if they do.

Comment 52

[Curt Patrick (gone)]

Since there are a handfull of considerations floating around still I'm reopening
this bug and I'll attach the patches.

Comment 53

[Curt Patrick (gone)]

Attachment: Relocating the xpi (moz)

Besides the one line I noted, there is also a comment line that needs modified.

Comment 54

[Curt Patrick (gone)]

Attachment: Relocating the xpi (ns)

Comment 55

[Daniel (Leaf) Nunes]

Comment on attachment 67114 [details] [diff] [review]
Relocating the xpi (moz)

r=leaf, i don't have an issue with this being created, i'm just not sure the
mozilla download page is going to make use of it (and therefore it might not
get delivered to ftp.mozilla.org).

Comment 56

[Daniel Veditz [:dveditz]]

Comment on attachment 67115 [details] [diff] [review]
Relocating the xpi (ns)

sr=dveditz

Comment 57

[Curt Patrick (gone)]

checked in the patches to move xpi wrapped installers up a directory.

Comment 58

[Grace Bush]

the N6Setup.xpi file is showing up in 
ftp://sweetlou.mcom.com/products/client/seamonkey/windows/32bit/x86/current/xpi/

and not where expected 
ftp://sweetlou.mcom.com/products/client/seamonkey/windows/32bit/x86/2002-02-01-0
6-trunk/ 

Is that where it should be and not with each build of the day?

Comment 59

[Curt Patrick (gone)]

No, this is not correct.  I notice that N6Setup.xpi is not showing up anywhere
in the dated trunk builds.  This is going to take the attention of our resident
build expert.  

Leaf, can you help me figure out what is going on?  If my latest checkin is
being used (on the trunk, not the branch) then I would expect this file to get
created in the same directory with N6Setup.exe, not in the xpi directory, and
then I think you were going to work some magic to get it delivered correctly. 
So I'm not sure where it is breaking down now.

Comment 60

[Grace Bush]

reopening- N6Setup.xpi only being delivered to  
ftp://sweetlou.mcom.com/products/client/seamonkey/windows/32bit/x86/current/xpi/

Comment 61

[Daniel (Leaf) Nunes]

i haven't worked the magic yet, is the problem.

Comment 62

[Curt Patrick (gone)]

In that case, we'll let Lead own this one.

Comment 63

[Curt Patrick (gone)]

Leaf, when you get done modifying the Windows build scripts please assign this
bug back to me (don't close it) since I just realized that this is a cross
platform bug and I've only done the Windows work (knucklehead that I am).

Comment 64

[Jon Granrose]

doesn't block 099 but has to get done for 1.0

Comment 65

[Daniel (Leaf) Nunes]

automation change is being tested; file should show up on sweetlou in less than
45 minutes.

Comment 66

[Daniel (Leaf) Nunes]

automation updated and committed. reassigning back to curt

Comment 67

[Grace Bush]

Curt, if you mark fixed, I will verify-this is working!

Comment 68

[Curt Patrick (gone)]

Yeah, but I still need to create the xpi's for unix and mac before I get to call
this done.

Comment 69

[Curt Patrick (gone)]

Turns out that xpi'ing up the mac and linux stub installers isn't going to be as
simple as it was for windows, because those two platforms do not have the
concept of a "self-extracting executable" so we have to package, install, launch
and then clean after ourselves.  This is going to be more work than we have time
for before the beta deadline.  Also, we do not need these for beta, they can be
created after beta.

So, I've created two new bugs: 130773 for mac & 130772 for linux.  I'm going to
close this bug (and I changed the summary to reflect the fact that this bug
deals only with the windows implementation.

Comment 70

[Grace Bush]

verified on 3/13 build

Comment 71

[Brant Gurganus]

[RFE] is deprecated in favor of severity: enhancement.  They have the same meaning.