Retention: take over user defaults during install on Win32

RESOLVED FIXED in Firefox 3

Status

()

Firefox
Installer
P4
normal
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

({late-l10n})

Trunk
Firefox 3
x86
Windows Vista
late-l10n
Points:
---
Dependency tree / graph
Bug Flags:
wanted-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 3 obsolete attachments)

See "Alter the default browser settings path for better user choice" at http://wiki.mozilla.org/Retention

1. Is this the first launch? (pref check?)
2. Is this the only profile for this user?
3. Is the app not set as the default?
4. Take defaults.

1. Is app launched from the OS? (url, applevent, etc. - won't be 100% detectable on all OS's)
2. Show UI as shown on wiki and act accordingly
http://wiki.mozilla.org/images/4/4e/Retention_Plan_2.010.png

note: comments about whether this is the right or wrong thing to do should be added to the following discussion page.
http://wiki.mozilla.org/Talk:Retention
Benjamin, I'd like your insight as to how feasible the steps above are?
> 4. Take defaults.

what does "Take defaults." mean?
It means to make Firefox the default handler for the protocols and files it can handle at the user level

Comment 4

11 years ago
Rob, let's talk in person, I'm not sure what the question is exactly. I don't particularly like making the behavior depend on whether you have one or more profiles installed.
Benjamin, definitely... I'll buy the drinks. :P

note: the reason for the check of the existence of other profiles is to prevent doing this for the same user if they have multiple profiles which can be leveraged by the community of testers. A better option might be to set this in profiles.ini or some other user specific place that would apply to all profiles.
> It means to make Firefox the default handler for the protocols and files it 
> can handle at the user level

Thanks for clarifying.  Comments here:  http://wiki.mozilla.org/Talk:Retention#concerns_over_taking_over_the_default

Comment 7

11 years ago
Can we touch base on the status of the bug this week? 

Also, I know there are some questions/concerns about this, so want to talk through that and figure out the best way to work through them.
Sure, I'll contact you probably Thursday. As for progress, no progress due to working on other bugs we need for Firefox 3.0 and 2.0.0.7

note: after discussing this with mconnor the current thought is to by default do this in the installer (win32 only) and take over the defaults when performing a default install and provide the option not to take over the defaults in an advanced install. The rationale is that installers such as media player installers already do this.

JT Batson, is that acceptable? Do we still need to display the additional ui to say we've taken over defaults?
Could you also get Retention added to the PRD since it is our working document for Firefox 3.0?

Comment 10

11 years ago
Retention in to PRD- sure-- how do I do that?

As for the solution you outlined above, I think that makes sense. Thanks for your help.
(In reply to comment #10)
> Retention in to PRD- sure-- how do I do that?
Touchbase with Basil or Beltzner if he wasn't on vacation

> As for the solution you outlined above, I think that makes sense.
OK, I'll start on this hopefully within the week.

Component: OS Integration → Installer
QA Contact: os.integration → installer
Created attachment 277861 [details]
installer screenshots

Hey Mike, since this has to be done just 'right' I want your take on this approach. I can hide the controls in the instance where an installation of Firefox is already set as the default browser. I can also add more install summary info.

btw: I've been wanting to add back the Summary wizard page (it was in the xpinstall wizard) for quite some time so we can change the next button to an install button just prior to the actual installation operations starting.
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Attachment #277861 - Flags: review?(mconnor)
note: I'm planning on keeping the check to see if the app is already default to only checking if we are the default handler for http. The app has thorough code for checking and this should suffice for this bug IMO.
OS: All → Windows Vista
Hardware: All → PC
Summary: Retention: take over user defaults at startup → Retention: take over user defaults during install on Win32

Updated

11 years ago
Attachment #277861 - Flags: review?(mconnor) → ui-review+
Target Milestone: --- → Firefox 3 M9
No longer blocks: 252273
Priority: -- → P4
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Created attachment 308034 [details] [diff] [review]
strings patch (checked in)
Attachment #308034 - Flags: review?(beltzner)
Attachment #308034 - Flags: approval1.9?
Keywords: late-l10n
Comment on attachment 308034 [details] [diff] [review]
strings patch (checked in)

only nits:

- "Use $BrandShortName as my default web browser"

also, I think that the checkbox should probably be aligned with the "Choose your..." label.

Other than that, looks great. You can make those changes at checkin.
Attachment #308034 - Flags: review?(beltzner)
Attachment #308034 - Flags: review+
Attachment #308034 - Flags: approval1.9?
Attachment #308034 - Flags: approval1.9+
Comment on attachment 308034 [details] [diff] [review]
strings patch (checked in)

Strings checked in to trunk
Checking in mozilla/browser/locales/en-US/installer/custom.properties;
/cvsroot/mozilla/browser/locales/en-US/installer/custom.properties,v  <--  custom.properties
new revision: 1.15; previous revision: 1.14
done
Attachment #308034 - Attachment description: strings patch → strings patch (checked in)
Attachment #308034 - Attachment is obsolete: true
bah... forgot to make the string change in original checkin
Checking in mozilla/browser/locales/en-US/installer/custom.properties;
/cvsroot/mozilla/browser/locales/en-US/installer/custom.properties,v  <--  custom.properties
new revision: 1.16; previous revision: 1.15
done
Maybe a spell check on param names... ;-)
SUMMARY_MAKE_DEAFULT
Since only the strings have landed, I'll correct the string name.
Keywords: checkin-needed
s/SUMMARY_MAKE_DEAFULT/SUMMARY_MAKE_DEFAULT/.

Checking in browser/locales/en-US/installer/custom.properties;
/cvsroot/mozilla/browser/locales/en-US/installer/custom.properties,v  <--  custom.properties
new revision: 1.17; previous revision: 1.16
done
Keywords: checkin-needed
Flags: wanted-firefox3+
bah... the name isn't displayed and it is better to not make l10n have to modify it just for that. oh well
(In reply to comment #19)
> Since only the strings have landed, I'll correct the string name.

Yeah, it's just localizers' time. Let's waste it. How long could it take, anyway? A minute at most, right?

But there are 45 locales, so it's really 45 minutes that could have been spent on sth slightly more important, like testing... :(
Created attachment 314485 [details] [diff] [review]
patch rev2

some cruft snuck in to the previous patch
Attachment #314483 - Attachment is obsolete: true
Attachment #314485 - Flags: review?(seth)
Attachment #314483 - Flags: review?(seth)
Comment on attachment 314485 [details] [diff] [review]
patch rev2

note: I didn't bother with the checks since we ask if the app should be take its handlers before we know the install dir so we can't test against the install location
Comment on attachment 314485 [details] [diff] [review]
patch rev2

also requesting from mconnor in case he can review this
Attachment #314485 - Flags: review?(mconnor)
Comment on attachment 314485 [details] [diff] [review]
patch rev2

also requesting from mconnor in case he has time to review this
Attachment #314485 - Flags: review?(benjamin)
Whiteboard: [has patch][need review]

Comment 28

10 years ago
Comment on attachment 314485 [details] [diff] [review]
patch rev2

I'm no NSIS expert, but this looks correct to me.
Attachment #314485 - Flags: review?(benjamin) → review+
Target Milestone: Firefox 3 beta2 → Firefox 3
Comment on attachment 314485 [details] [diff] [review]
patch rev2

Requesting a1.9. This patch uses the existing code for setting Firefox as the default browser and is pretty darn safe.

Also, Seth said he should be able to review the patch this afternoon so we will likely have a third set of eyes on it.
Attachment #314485 - Flags: review?(mconnor) → approval1.9?
Whiteboard: [has patch][need review] → [has patch]

Updated

10 years ago
Attachment #314485 - Flags: review?(seth) → review+
Attachment #314485 - Flags: approval1.9? → approval1.9+
Created attachment 314937 [details]
updated screenshots for all states
Attachment #277861 - Attachment is obsolete: true
Thanks for the reviews Benjamin and Seth!

Checked in to trunk

Checking in mozilla/browser/installer/windows/nsis/installer.nsi;
/cvsroot/mozilla/browser/installer/windows/nsis/installer.nsi,v  <--  installer.nsi
new revision: 1.42; previous revision: 1.41
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh,v  <--  common.nsh
new revision: 1.40; previous revision: 1.39
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [has patch]

Updated

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