The default bug view has changed. See this FAQ.

Add a silent install option for Mozilla Maintenance Service

RESOLVED FIXED in Firefox 16

Status

()

Firefox
Installer
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: hectorz, Assigned: hectorz)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

unspecified
Firefox 16
x86
Windows 7
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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
(Assignee)

Comment 1

5 years ago
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.
Duplicate of this bug: 761108
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
Attachment #617803 - Flags: review?(netzen)
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.
Attachment #617803 - Flags: review?(netzen) → review-
Assignee: nobody → bzhao
(Assignee)

Comment 5

5 years ago
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.
Attachment #617803 - Attachment is obsolete: true
Attachment #632988 - Flags: review?(netzen)
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!
Attachment #632988 - Flags: review?(netzen) → review+
Target Milestone: --- → Firefox 16
I updated the documentation here:
https://wiki.mozilla.org/Installer:Command_Line_Arguments
Keywords: dev-doc-complete
(Assignee)

Comment 8

5 years ago
(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

5 years ago
https://bugzilla.mozilla.org/show_bug.cgi?id=764709
Blocks: 764709
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
Status: NEW → ASSIGNED
(Assignee)

Comment 11

5 years ago
Thank you for helping me get my first patch landed!
https://hg.mozilla.org/mozilla-central/rev/ea52f4eb380c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 765596
You need to log in before you can comment on or make changes to this bug.