741174 - Add a silent install option for Mozilla Maintenance Service

Status: RESOLVED FIXED

(Firefox :: Installer, defect)

Description

[Hector Zhao [:hectorz]]

User can silently install Firefox and customize install path and shortcuts using a configuration ini file[1], and Firefox will never be set as default browser during silent install[2].

Mozilla Maintenance Service is now a customizable component in the custom install process. In the silent install process, whether it will be installed is based on platform and user priviledge, and cannot be controlled with the ini file.

The network installer distributed by Beijing office pass user install options to the background downloaded Firefox installer using a generated ini file, so we need this to give user control over the installation of the maintenance service.

Thanks!

[1]: https://wiki.mozilla.org/Installer:Command_Line_Arguments
[2]: https://bugzilla.mozilla.org/show_bug.cgi?id=440704

Comment 1

[Hector Zhao [:hectorz]]

Attachment: Patch, temporary silent install option for maintenance service

A patched version of Firefox 12 installer is distributed by Beijing office as a temporary solution. Attached is the said patch.

Comment 3

[Brian R. Bondy [:bbondy]]

Comment on attachment 617803 [details] [diff] [review]
Patch, temporary silent install option for maintenance service

Marking myself to review this so we can get it landed

Comment 4

[Brian R. Bondy [:bbondy]]

Comment on attachment 617803 [details] [diff] [review]
Patch, temporary silent install option for maintenance service

Thanks for the patch Hector, would you mind doing one more pass with the below comments implemented? Then it should be good to land.

The problem with having a "${If} ${Errors}" check here is if any of the other optional options are not present, the MaintenanceService option will not be used.  What I mean is any ReadINIStr can set an error, not just the MaintenanceService check. 

We should instead just take off the "${If} ${Errors}" check here since we just clear the errors after anyway.

Comment 5

[Hector Zhao [:hectorz]]

Attachment: Patch v2

Thanks for the review, Brian!

I think worries back then was, if users reuse their locally saved earlier version of network installer, an ini file with no MaintenanceService entry will be passed to the newly downloaded Firefox installer, and I'm not sure if I can safely check $R8 in such situation.

Comment 6

[Brian R. Bondy [:bbondy]]

Comment on attachment 632988 [details] [diff] [review]
Patch v2

Review of attachment 632988 [details] [diff] [review]:
-----------------------------------------------------------------

Yup if you're using an old build that doesn't have the new code for "MaintenanceService" then it'll just put an empty string for $R8 so I think this is good as is.  We call ClearErrors anyway after the /INI= block.  

Do you have L3 push access already? If so please land away.  If not I can land this for you.

Thanks for the patch and the quick implementation of the review comments!

Comment 7

[Brian R. Bondy [:bbondy]]

I updated the documentation here:
https://wiki.mozilla.org/Installer:Command_Line_Arguments

Comment 8

[Hector Zhao [:hectorz]]

(In reply to Brian R. Bondy [:bbondy] from comment #6)
> Do you have L3 push access already? If so please land away.  If not I can
> land this for you.
> 
I don't have commit access, so please help me land this, thanks!

Comment 9

[tomorrow]

https://bugzilla.mozilla.org/show_bug.cgi?id=764709

Comment 10

[Brian R. Bondy [:bbondy]]

Pushed to m-i, someone will migrate that to mozilla-central in around a day.  Thanks again for the patch!

http://hg.mozilla.org/integration/mozilla-inbound/rev/ea52f4eb380c

Comment 11

[Hector Zhao [:hectorz]]

Thank you for helping me get my first patch landed!

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ea52f4eb380c