572405 - Installer should read defaults from a setup.ini file

Status: RESOLVED WONTFIX

(Firefox :: Installer, defect)

Description

[Mike Kaply [:mkaply]]

Attachment: Read checkboxes from built in INI file

It would be nice for creating custom installers if the defaults for the various checkboxes could be read from an INI file that was contained in the install versus specifying on the command line.

Also these defaults would be used by the wizard.

The patch attached does just that.

It checks for a file called setup.ini and if found, reads the default values for the checkboxes from that file.

In order to do this, I had to change installer.ini so that it uses those variables when writing the INI files used by the various pages.

Note I did not add support for everything in the INI file into the setup.ini file.

For my purposes, I didn't need everything. This could be done trivially.

Comment 1

[Robert Strong (they/them - no direct email)]

Comment on attachment 451585 [details]
Read checkboxes from built in INI file

I'm fine with this with the following changes... will still need to review after the changes have been made.

1. It should use the setup.ini in the distributions directory that is currently used for branding instead of another ini file.

2. Check for the /INI command line argument (it should override the setup.ini) and if not present then check for the existence of the distributions\setup.ini. Set a var to the ini file and use the same ReadINIStr calls.

3. I'd prefer not having the option to set as the default browser when running silent since there will most likely be admins that *think* it should set it as the default for the users of the system instead of the account they are using to install with. I can get over that and just document this fact.

Comment 2

[Mike Kaply [:mkaply]]

Where is the distributions directory in an unpacked build? It is at the same level as localized/nonlocalized/optional?

Or is it in one of the subdirectories?

Thanks

Comment 3

[Robert Strong (they/them - no direct email)]

$EXEDIR\localized\distribution\setup.ini

http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/installer.nsi#951

Comment 4

[Mike Kaply [:mkaply]]

Attachment: Use INI command line if present

I apologize if I screwed up tabs in this file. It appears to use a mix of tabs and spaces.

In my new code, I used two space indentation.

Comment 5

[Mike Kaply [:mkaply]]

Also, where is the documentation on what can be in the setup.ini used by the installer? I'm modifying the installer files directly and would love to find another way.

Comment 6

[Robert Strong (they/them - no direct email)]

(In reply to comment #4)
> Created an attachment (id=451605) [details]
> Use INI command line if present
> 
> I apologize if I screwed up tabs in this file. It appears to use a mix of tabs
> and spaces.
> 
> In my new code, I used two space indentation.
Just did a search of common.nsh and there isn't a single tab in the file so it must be something you did to your source.

The setup.ini was requested for distributions and it never went anywhere hence no docs... take a look at the SetBrandNameVars in common.nsh for more info.

You can also change the images in the installer... take a look at the preWelcome and preOptions functions in installer.nsi

Comment 7

[Mike Kaply [:mkaply]]

OK, looking again, I think that patch is good. The whitespace movement is just due to indentation movement because of the nested ifs.

Would you rather see a patch with no whitespace at all?

Comment 8

[Robert Strong (they/them - no direct email)]

I can manage... just to set expectations, I should be able to get to the review by next week.

Comment 9

[Mike Kaply [:mkaply]]

FYI, I'm not rushed on this. I'm having to build the installer custom for my purposes anyway.

Comment 10

[Mike Kaply [:mkaply]]

Forgot about this one. Any chance of this making FF4?

Comment 11

[Mike Kaply [:mkaply]]

Robert, any time to look at this?

Comment 12

[Mike Kaply [:mkaply]]

Robert, any time for this?

Comment 13

[Ed Morley [:emorley]]

(Robert wasn't CC'd)

Comment 14

[John]

Curious if there were any updates on this? It appears that there is a fix available, just waiting to be committed. 

Thanks in advance.

Comment 16

[Robert Strong (they/them - no direct email)]

Comment on attachment 451605 [details] [diff] [review]
Use INI command line if present

(In reply to John from comment #14)
> Curious if there were any updates on this? It appears that there is a fix
> available, just waiting to be committed. 
> 
> Thanks in advance.
The porting of the stub installer code to the complete installer takes precedence over this and this patch will likely make that work more complicated.

Besides mkaply, how many people really want the ability to specify the defaults during a non-administrative install? What is the use case.

minusing for some obvious formatting errors.

Comment 17

[Mike Kaply [:mkaply]]

This was requested by some enterprises because our installer isn't easily customizable. This would allow them to customize any of our installs.

But that was a long time ago. I think by this point, most enterprises have given up on using our installer.