Last Comment Bug 741174 - Add a silent install option for Mozilla Maintenance Service
: Add a silent install option for Mozilla Maintenance Service
Status: RESOLVED FIXED
: dev-doc-complete
Product: Firefox
Classification: Client Software
Component: Installer (show other bugs)
: unspecified
: x86 Windows 7
: -- normal (vote)
: Firefox 16
Assigned To: Hector Zhao [:hectorz]
:
Mentors:
: 761108 (view as bug list)
Depends on: 765596
Blocks: 764709
  Show dependency treegraph
 
Reported: 2012-04-01 02:53 PDT by Hector Zhao [:hectorz]
Modified: 2012-06-17 11:15 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, temporary silent install option for maintenance service (1.07 KB, patch)
2012-04-23 23:51 PDT, Hector Zhao [:hectorz]
netzen: review-
Details | Diff | Splinter Review
Patch v2 (1021 bytes, patch)
2012-06-13 18:46 PDT, Hector Zhao [:hectorz]
netzen: review+
Details | Diff | Splinter Review

Description Hector Zhao [:hectorz] 2012-04-01 02:53:25 PDT
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] 2012-04-23 23:51:03 PDT
Created attachment 617803 [details] [diff] [review]
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 2 Brian R. Bondy [:bbondy] 2012-06-11 06:04:06 PDT
*** Bug 761108 has been marked as a duplicate of this bug. ***
Comment 3 Brian R. Bondy [:bbondy] 2012-06-13 07:11:00 PDT
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] 2012-06-13 13:02:15 PDT
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] 2012-06-13 18:46:23 PDT
Created attachment 632988 [details] [diff] [review]
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] 2012-06-13 19:29:30 PDT
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] 2012-06-13 19:35:24 PDT
I updated the documentation here:
https://wiki.mozilla.org/Installer:Command_Line_Arguments
Comment 8 Hector Zhao [:hectorz] 2012-06-13 19:43:32 PDT
(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 10 Brian R. Bondy [:bbondy] 2012-06-15 05:11:06 PDT
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] 2012-06-15 07:45:45 PDT
Thank you for helping me get my first patch landed!
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-06-16 06:53:06 PDT
https://hg.mozilla.org/mozilla-central/rev/ea52f4eb380c

Note You need to log in before you can comment on or make changes to this bug.