Bootstrapper

Bootstrap installer.

Overrides the UI of its packaged MSI's and substitutes its own. Localizable, at least if the quasi-documentation is accurate (see source comment).

All the localization strings are in the ".wxl" file.

Supersedes http://codereview.adblockplus.org/10977019/

[Felix Dahlke, 2013-08-01 09:51:21]

http://codereview.adblockplus.org/11258006/diff/1/installer/1031/bootstrap-th...
File installer/1031/bootstrap-theme.wxl (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/1031/bootstrap-th...
installer/1031/bootstrap-theme.wxl:2: <WixLocalization Culture="de-de"
Language="1033" xmlns="http://schemas.microsoft.com/wix/2006/localization">
Shouldn't this be Language="1031"?

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.wxl
File installer/bootstrap-theme.wxl (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:20: <String Id="InstallWelcome">This setup program
installs Adblock Plus for IE into your "Program Files" folder in directory
"Adblock Plus for IE".</String>
I think this sounds a bit awkward, can't we have something like: This setup
program installs Adblock Plus for IE to "%PROGRAM_FILES%/Adblock Plus for IE" Or
maybe just: This setup program installs Adblock Plus for IE to your "Program
Files" directory.

http://codereview.adblockplus.org/11258006/diff/1/installer/setup.wxs
File installer/setup.wxs (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/setup.wxs#newcode38
installer/setup.wxs:38: Localization payloads. See
http://stackoverflow.com/a/11402260/1794538 for it's done this way.
"for *why* it's done this way"?

[Wladimir Palant, 2013-08-01 10:30:33]

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.wxl
File installer/bootstrap-theme.wxl (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:6: <String Id="ConfirmCancelMessage">Are you sure
you want to cancel?</String>
I think this is the more common way to phrase it:

Are you sure you wish to abort installation?

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:20: <String Id="InstallWelcome">This setup program
installs Adblock Plus for IE into your "Program Files" folder in directory
"Adblock Plus for IE".</String>
On 2013/08/01 09:51:21, Felix H. Dahlke wrote:
> I think this sounds a bit awkward, can't we have something like: This setup
> program installs Adblock Plus for IE to "%PROGRAM_FILES%/Adblock Plus for IE"
Or
> maybe just: This setup program installs Adblock Plus for IE to your "Program
> Files" directory.

I don't think we should refer to environment variables here, our users won't
know about them. So I would go with your second suggestion:

This setup program installs [WixBundleName] to your "Program Files" directory.

Note that we should use WixBundleName consistently.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:22: <String Id="InstallCloseButton">&amp;Do not
install</String>
The user doesn't choose between installing and not installing - that choice has
been made already, Adblock Plus should be installed. So the second button should
have a generic name, "Cancel" is the usual one IMHO. Also, I think that it
should be called that on all screens (meaning no separate ProgressCancelButton
string).

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:35: <String Id="ModifyCloseButton">&amp;Do
nothing</String>
Again, I would rather have a generic "Cancel" button here.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:39: <String
Id="SuccessLaunchButton">&amp;Launch</String>
Launch what exactly? This should probably say: "Start Internet Explorer"

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:42: <String
Id="SuccessCloseButton">&amp;Done</String>
If we are asking the user to restart then the options should read "Restart Now"
and "Restart Later" IMHO.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:49: <String
Id="FailureCloseButton">&amp;Done</String>
Same here, "Restart Now" and "Restart Later" in case of restart IMHO.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.xml
File installer/bootstrap-theme.xml (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.x...
installer/bootstrap-theme.xml:1: <?xml version="1.0" encoding="utf-8"?>
I cannot review this file - it's a horrible mess with tons of hardcoded
coordinates that will likely fall apart in the very first translation we get :-(

[Felix Dahlke, 2013-08-01 10:52:43]

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.wxl
File installer/bootstrap-theme.wxl (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:20: <String Id="InstallWelcome">This setup program
installs Adblock Plus for IE into your "Program Files" folder in directory
"Adblock Plus for IE".</String>
On 2013/08/01 10:30:33, Wladimir Palant wrote:
> On 2013/08/01 09:51:21, Felix H. Dahlke wrote:
> > I think this sounds a bit awkward, can't we have something like: This setup
> > program installs Adblock Plus for IE to "%PROGRAM_FILES%/Adblock Plus for
IE"
> Or
> > maybe just: This setup program installs Adblock Plus for IE to your "Program
> > Files" directory.
> 
> I don't think we should refer to environment variables here, our users won't
> know about them. So I would go with your second suggestion:
> 
> This setup program installs [WixBundleName] to your "Program Files" directory.
> 
> Note that we should use WixBundleName consistently.

Well, I meant to actually show the full path to the target folder,
%PROGRAM_FILES% isn't even a variable. But yeah, might be better to go with the
simpler second variant.

[Eric, 2013-08-01 18:58:37]

http://codereview.adblockplus.org/11258006/diff/1/installer/1031/bootstrap-th...
File installer/1031/bootstrap-theme.wxl (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/1031/bootstrap-th...
installer/1031/bootstrap-theme.wxl:2: <WixLocalization Culture="de-de"
Language="1033" xmlns="http://schemas.microsoft.com/wix/2006/localization">
On 2013/08/01 09:51:21, Felix H. Dahlke wrote:
> Shouldn't this be Language="1031"?
Yes. Missed this one.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.wxl
File installer/bootstrap-theme.wxl (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:6: <String Id="ConfirmCancelMessage">Are you sure
you want to cancel?</String>
The problem is that there's only one cancellation dialog no matter what you're
cancelling: (1) initial installation dialog or (2) repair/uninstall dialog. It's
a defect, IMO, that these use the same string. But fixing it means rewriting the
whole UI code.

I left it generic so that it basically fits both situations.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:20: <String Id="InstallWelcome">This setup program
installs Adblock Plus for IE into your "Program Files" folder in directory
"Adblock Plus for IE".</String>
On 2013/08/01 09:51:21, Felix H. Dahlke wrote:
>"%PROGRAM_FILES%/Adblock Plus for IE"

The biggest problem with this message is that there's no way (not that I could
find) to give a different message based on a 32-bit or 64-bit platform. So a
full technical disclosure is impossible with the current tool.

Furthermore, I'm not ever sure that %...% substitution is even supported. The
only reason I even know about the [...] substitution is that it was already used
in the example code. There's no documentation about the syntax.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:20: <String Id="InstallWelcome">This setup program
installs Adblock Plus for IE into your "Program Files" folder in directory
"Adblock Plus for IE".</String>
On 2013/08/01 10:30:33, Wladimir Palant wrote:
> This setup program installs [WixBundleName] to your "Program Files" directory

I'll do that. It also appears again below.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:22: <String Id="InstallCloseButton">&amp;Do not
install</String>
On 2013/08/01 10:30:33, Wladimir Palant wrote:
> So the second button should
> have a generic name, "Cancel" is the usual one IMHO. 

"Cancel" it is.

> Also, I think that it
> should be called that on all screens (meaning no separate ProgressCancelButton
> string).

Sorry, can't change the string names. These are names hard-wired in the C# UI
code.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:39: <String
Id="SuccessLaunchButton">&amp;Launch</String>
On 2013/08/01 10:30:33, Wladimir Palant wrote:
> Launch what exactly? This should probably say: "Start Internet Explorer"

This is the generic text. It's not invoked at present. I left the definition
here in case we use it. If we end up needing it, then what you've suggested is
probably the right text.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.w...
installer/bootstrap-theme.wxl:42: <String
Id="SuccessCloseButton">&amp;Done</String>
Again, these are not used.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.xml
File installer/bootstrap-theme.xml (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.x...
installer/bootstrap-theme.xml:1: <?xml version="1.0" encoding="utf-8"?>
On 2013/08/01 10:30:33, Wladimir Palant wrote:
> I cannot review this file - it's a horrible mess with tons of hardcoded
> coordinates that will likely fall apart in the very first translation we get
:-(

I completely agree, which is why I didn't want to touch the thing at all. If you
were wondering why I was squirming about dealing with the 'burn' UI, this is
why. It's bloody awful. FYI, this is pretty much directly analogous to the
default UI in Windows Installer. Why they felt it necessary to propagate such
filth is beyond me.

The only way I've ever seen people deal with the sizing issues given these tools
is just to use enough white space to allow the most verbose languages. So
there's not much choice but to do the translations and manually check that they
all fit.

http://codereview.adblockplus.org/11258006/diff/1/installer/setup.wxs
File installer/setup.wxs (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/setup.wxs#newcode38
installer/setup.wxs:38: Localization payloads. See
http://stackoverflow.com/a/11402260/1794538 for it's done this way.
Yes.

[Oleksandr, 2013-08-03 17:51:39]

LGTM in general, apart of control sizes hardcoded, of course.

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.xml
File installer/bootstrap-theme.xml (right):

http://codereview.adblockplus.org/11258006/diff/1/installer/bootstrap-theme.x...
installer/bootstrap-theme.xml:1: <?xml version="1.0" encoding="utf-8"?>
Yeah. This was something I've noticed right away as well. 
Looks rather awful, but I remember I have seen same kind of bugs (trimmed text
in buttons) quite a lot, including in some of the Microsoft's products. I guess
we'll have to ask translators to be brief :/ 
On 2013/08/01 18:58:37, Eric wrote:
> On 2013/08/01 10:30:33, Wladimir Palant wrote:
> > I cannot review this file - it's a horrible mess with tons of hardcoded
> > coordinates that will likely fall apart in the very first translation we get
> :-(
> 
> I completely agree, which is why I didn't want to touch the thing at all. If
you
> were wondering why I was squirming about dealing with the 'burn' UI, this is
> why. It's bloody awful. FYI, this is pretty much directly analogous to the
> default UI in Windows Installer. Why they felt it necessary to propagate such
> filth is beyond me.
> 
> The only way I've ever seen people deal with the sizing issues given these
tools
> is just to use enough white space to allow the most verbose languages. So
> there's not much choice but to do the translations and manually check that
they
> all fit.

[Felix Dahlke, 2013-08-05 13:06:23]

Eric, as discussed elsewhere, can you check that this works with texts which are
roughly twice as long? If that works, then I guess we can go with the fixed
sizes for now.

If it doesn't work, we will only release an English installer and look into
localised installers later. We'll still want to have a single installer that
starts the correct MSI, so we might as well go with the bootstrapper.