Closed Bug 1095739 Opened 10 years ago Closed 8 years ago

Allow a "new user" experience to happen subsequent to Firefox being uninstalled

Categories

(Firefox :: General, defect, P2)

36 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed

People

(Reporter: verdi, Assigned: jaws)

References

(Depends on 1 open bug, )

Details

(Whiteboard: [fxgrowth][Onboarding])

Attachments

(2 files, 1 obsolete file)

Since we always leave the user profile behind when uninstalling Firefox, when a user tries Firefox again they don't get a new user experience. This results in experiences where they aren't told about any new features and/or start with unwanted add-ons, hijacked search, home or new tab pages. 

Examples: https://www.usertesting.com/highlight_reels/k2MSj89J13Ked9NtftJJ

We should provide some way to automatically give these users a new user experience after re-installing Firefox.
One solution might be to perform a profile refresh as part of the uninstallation process.
Blocks: 1062896
(In reply to Verdi [:verdi] from comment #0)
> Since we always leave the user profile behind when uninstalling Firefox,
> when a user tries Firefox again they don't get a new user experience.

I believe we offer a checkbox to delete the data so "always" isn't really correct.
(In reply to Matthew N. [:MattN] (focused on Loop) from comment #2)
> (In reply to Verdi [:verdi] from comment #0)
> > Since we always leave the user profile behind when uninstalling Firefox,
> > when a user tries Firefox again they don't get a new user experience.
> 
> I believe we offer a checkbox to delete the data so "always" isn't really
> correct.

We seemed to have gotten rid of that a few releases ago (not sure when exactly).
(In reply to Verdi [:verdi] from comment #3)
> (In reply to Matthew N. [:MattN] (focused on Loop) from comment #2)
> > (In reply to Verdi [:verdi] from comment #0)
> > > Since we always leave the user profile behind when uninstalling Firefox,
> > > when a user tries Firefox again they don't get a new user experience.
> > 
> > I believe we offer a checkbox to delete the data so "always" isn't really
> > correct.
> 
> We seemed to have gotten rid of that a few releases ago (not sure when
> exactly).

bug 432017.

Do we have a way to detect a renewed install? Matt?
Flags: needinfo?(MattN+bmo)
We should include old or stale profiles as part of this user story since not everyone uninstalls when they move to another browser.
Easy algorithm (imperfect, but this could at least drive a prompt):

* check the time stamp on prefs.js and/or other files
* check the value of various prefs (time stamps for updater, etc)

This isn't super hard to build, especially if we want to have something a little more sophisticated than a simple new user experience.  Something like:

Welcome Back!  It's been a while since we last saw you, and we wanted to give you a clean slate with the new Firefox.  If you want your old bookmarks/add-ons/etc, click here to pick what you want to restore.

If they have a Firefox Account configured, we can go one better and do a full sync into an empty profile.

This is an issue for partner distributors, so a fix would be great ASAP.
(In reply to Chris More [:cmore] from comment #6)
> We should include old or stale profiles as part of this user story since not
> everyone uninstalls when they move to another browser.

We already show a notification when you start to use a profile that hasn't been used for a long time. I believe the difference here is that it's about reinstalls and that it's about potentially starting with a completely new profile rather than using Firefox refresh/reset. It probably makes the most sense for Verdi to clarify exactly what he has in mind and how it differs from what we currently do.
Priority: -- → P4
Whiteboard: [fxgrowth] → [fxgrowth][Onboarding]
Priority: P4 → P2
I think this bug is not ready for engineering to fix. It needs:

- a UX decision about when we do this. For pave-over installs? Only if the uninstaller ran first? Only after a certain amount of time has passed? (ie what if I uninstall, download a new copy using IE, reinstall, all in the space of an hour/day/week/month). What about non-Windows where there's no installer?
--- note also that we already do stuff if you start Firefox (reinstalled or otherwise) with a profile that hasn't been used in months (I think the threshold is 3 or 6 months but I could be wrong.) Do we really need something else here?

- a UX decision about whether this is automatic (tadah, we reset your stuff) or a choice thing ("We found your Firefox data from <N> days/weeks/months ago, do you want to use that or start fresh?"), or only migrate some things (a la Firefox Refresh, but maybe without the homepage setting or something, if we're worried about hijacking?)

- based on that, we'll likely need a design that we can implement to tell the user what happened / what their choices are.

- clarification about what the "partner issue" here is (comment #7). It's not clear to me from that comment how this impacts partners and whether the best fix for them is necessarily what this bug was filed to. As for sync, see bug 670989.

Some personal thoughts here relating purely to comment #0 being the original intent here:

* if you install 43, uninstall 2 weeks later, install 44 6 weeks after that, you should already get whatever tours/new things you would normally get for an upgrade to 44 (from 43). I don't think it is the case that people "aren't told about any new features". If there are cases where this doesn't happen, we should file specific bugs about some of that (though it might be too late for those features, we should still figure out what is going on there). Unless this is about skipping versions (e.g. update from 41 to 44 you miss whatever tour we used for 42 -- but so would a new install of 44!) in which case I don't really understand what the goal is. Showing users all the tours from v4 to v45 would not really work very well...

* hijacked/add-on-ified home/search/newtab are all being combated in other ways, and that can be done without modifying the installer or trying to deal specifically with the uninstall/reinstall case. Is fixing just the uninstall/reinstall case really the best use of effort/time? Edge/IE have chosen the "kill all the BHO/ActiveX things" route, and we're doing the same with plugins. Yes, we should keep the ability to customize Firefox as people like, but it seems simply becoming more aggressive about what is allowed in this space can be done without expending effort to optimize uninstall/reinstall flow and would be more effective.

* do we have data on how many people fall into this bucket of uninstalling+reinstalling? And are we doing this for the users who do this in an effort to 'fix' hijacking, or purely for people who now get "old junk they forgot about" from 6 weeks/months/years ago? How many people are in those buckets?
Flags: needinfo?(mverdi)
Flags: needinfo?(mconnor)
Flags: needinfo?(MattN+bmo)
(In reply to :Gijs Kruitbosch from comment #9)
> I think this bug is not ready for engineering to fix. It needs:
> 
> - a UX decision about when we do this. For pave-over installs? Only if the
> uninstaller ran first? Only after a certain amount of time has passed? (ie
> what if I uninstall, download a new copy using IE, reinstall, all in the
> space of an hour/day/week/month). What about non-Windows where there's no
> installer?

Attached is a flowchart to help explain/define my proposal.
This would only be for Windows.
There are two scenarios:
1. Uninstalled – any time the user uninstalls Firefox and then reinstalls it, they’re either trying to fix a problem or they’ve decided to give Firefox another shot. Refresh was designed for these use-cases so auto-refreshing should be helpful.
2. Unused – if the user hasn’t used Firefox for more than 60 days and doesn’t have it set as the default browser, they’re not really a Firefox user. So in this case we want to give them a like-new experience.


> --- note also that we already do stuff if you start Firefox (reinstalled or
> otherwise) with a profile that hasn't been used in months (I think the
> threshold is 3 or 6 months but I could be wrong.) Do we really need
> something else here?

It’s currently 60 days. We show an infobar at the bottom of the screen suggesting that the user refresh Firefox. This often goes unnoticed/not acted upon and even when used it only provides part of the solution (in this case missing the ability to display a special what’s new page).


> 
> - a UX decision about whether this is automatic (tadah, we reset your stuff)
> or a choice thing ("We found your Firefox data from <N> days/weeks/months
> ago, do you want to use that or start fresh?"), or only migrate some things
> (a la Firefox Refresh, but maybe without the homepage setting or something,
> if we're worried about hijacking?)

We want to do an automatic refresh. The rationale is that these are: users trying to fix an issue (in this case let’s try to fix it), or users giving us a second chance. The refresh feature was made for these purposes and since we are going to migrate add-ons (disabled see Bug 1017919) there are no more reasons to not do this automatically. 

> 
> - based on that, we'll likely need a design that we can implement to tell
> the user what happened / what their choices are.

This may not be necessary. I’d like this interaction to be automatic and then put people into a new user experience or a special what’s new experience. What happened info (if necessary) might be addressed there. I’ll have to give that more consideration. 


> Some personal thoughts here relating purely to comment #0 being the original
> intent here:
> 
> * if you install 43, uninstall 2 weeks later, install 44 6 weeks after that,
> you should already get whatever tours/new things you would normally get for
> an upgrade to 44 (from 43). I don't think it is the case that people "aren't
> told about any new features". If there are cases where this doesn't happen,
> we should file specific bugs about some of that (though it might be too late
> for those features, we should still figure out what is going on there).
> Unless this is about skipping versions (e.g. update from 41 to 44 you miss
> whatever tour we used for 42 -- but so would a new install of 44!) in which
> case I don't really understand what the goal is. Showing users all the tours
> from v4 to v45 would not really work very well...

In this scenario, the user would only see something if we had built a what’s new page (designed for Firefox users who are updating) for this update. We try to do this a little as possible. In this case the user is not really a Firefox user so we want to make sure they get a like-new experience and not a just-updating-to-the-latest-version experience. I don’t think the page we’ll want to display should simply be a list of new features or a collection of the previous what’s new pages. It probably should also address this special context. If we auto-refresh the profile that exists on the computer, we can potentially fix issues that had them uninstall while saving any helpful data they may have collected in the time that they tried out Firefox.


> 
> * hijacked/add-on-ified home/search/newtab are all being combated in other
> ways, and that can be done without modifying the installer or trying to deal
> specifically with the uninstall/reinstall case. Is fixing just the
> uninstall/reinstall case really the best use of effort/time? Edge/IE have
> chosen the "kill all the BHO/ActiveX things" route, and we're doing the same
> with plugins. Yes, we should keep the ability to customize Firefox as people
> like, but it seems simply becoming more aggressive about what is allowed in
> this space can be done without expending effort to optimize
> uninstall/reinstall flow and would be more effective.


Those are a large subset but not all of the issues fixed by refreshing Firefox. In addition, any fixes in progress for these issues are fixes, I believe, that happen while using Firefox. In this scenario, we want Firefox to be fixed by the time it opens without user interaction. People are giving us a second chance so it’s especially important to give them an exceptional experience from the very beginning.


> 
> * do we have data on how many people fall into this bucket of
> uninstalling+reinstalling? And are we doing this for the users who do this
> in an effort to 'fix' hijacking, or purely for people who now get "old junk
> they forgot about" from 6 weeks/months/years ago? How many people are in
> those buckets?

We’re doing this because we think we think the current experience is bad ( https://docs.google.com/a/mozilla.com/presentation/d/1_W-V4EvtEA_B2fgtK-q2jVM5t4l1CwruBAG9wf0dfAA/edit?usp=sharing see slides 11 and 12 – double-click for video on 12 – LDAP required because of PII) and we think we can encourage people to use Firefox more by fixing it.
A lower estimate for how many people uninstall and then reinstall Firefox (no matter the time in between uninstall and reinstall) is about 10% of people installing. I don’t have an estimate for how many people open a profile that is more than 60 days old.
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #10)
> Created attachment 8720448 [details]
> Returning users flowchart
> 
> (In reply to :Gijs Kruitbosch from comment #9)
> > I think this bug is not ready for engineering to fix. It needs:
> > 
> > - a UX decision about when we do this. For pave-over installs? Only if the
> > uninstaller ran first? Only after a certain amount of time has passed? (ie
> > what if I uninstall, download a new copy using IE, reinstall, all in the
> > space of an hour/day/week/month). What about non-Windows where there's no
> > installer?
> 
> Attached is a flowchart to help explain/define my proposal.
> This would only be for Windows.
> There are two scenarios:
> 1. Uninstalled – any time the user uninstalls Firefox and then reinstalls
> it, they’re either trying to fix a problem or they’ve decided to give
> Firefox another shot. Refresh was designed for these use-cases so
> auto-refreshing should be helpful.

OK, so this needs installer work.

> 2. Unused – if the user hasn’t used Firefox for more than 60 days and
> doesn’t have it set as the default browser, they’re not really a Firefox
> user. So in this case we want to give them a like-new experience.

OK, so even in the 60-days-unused case you want to automatically refresh (rather than give them the choice but not always do it)? But only in the case where Firefox isn't the default browser (based on the flowchart you attached)?

> This often goes unnoticed/not
> acted upon and even when used it only provides part of the solution (in this
> case missing the ability to display a special what’s new page).

OK, so we also need a special what's new page. Note that we already have the 'about:welcomeback' page which lets people restore tabs and continue on their way. We could modify that. Opening a secondary page in addition to that page seems like it might not be a great experience / very useful.

> The refresh feature was made for these purposes and
> since we are going to migrate add-ons (disabled see Bug 1017919) there are
> no more reasons to not do this automatically. 

Well, users' preferences don't get migrated, either...

> I don’t think the page
> we’ll want to display should simply be a list of new features or a
> collection of the previous what’s new pages. It probably should also address
> this special context.

This sounds exactly like about:welcomeback . Is it supposed to be different?

All in all this sounds like a large change that should be split off into several separate bugs. We also need more clarity on how the 'what's new' page should work, and if it should also show up if a user manually resets Firefox, if it's in addition to about:welcomeback or should be integrated in there somehow, etc.

I also think we need to fix bug 1017919 and bug 1054947 (and potentially other blockers of bug 498181) before enabling auto-reset in the 60-days case.

Does that make sense?
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #10)
> > --- note also that we already do stuff if you start Firefox (reinstalled or
> > otherwise) with a profile that hasn't been used in months (I think the
> > threshold is 3 or 6 months but I could be wrong.) Do we really need
> > something else here?
> 
> It’s currently 60 days. We show an infobar at the bottom of the screen
> suggesting that the user refresh Firefox. This often goes unnoticed/not
> acted upon and even when used it only provides part of the solution (in this
> case missing the ability to display a special what’s new page).
> 
> 
> > 
> > - a UX decision about whether this is automatic (tadah, we reset your stuff)
> > or a choice thing ("We found your Firefox data from <N> days/weeks/months
> > ago, do you want to use that or start fresh?"), or only migrate some things
> > (a la Firefox Refresh, but maybe without the homepage setting or something,
> > if we're worried about hijacking?)
> 
> We want to do an automatic refresh. The rationale is that these are: users
> trying to fix an issue (in this case let’s try to fix it), or users giving
> us a second chance. The refresh feature was made for these purposes and
> since we are going to migrate add-ons (disabled see Bug 1017919) there are
> no more reasons to not do this automatically. 

Showing the infobar after uninstall-reinstall will still be a net improvement and increase the utility of the code that we already have for doing this.

If we think the current infobar is too easy to miss, we may want to find ways to improve its visibility. Improving the visibility would help for this as well as the not-used-in-60-days case.
Taking to implement a simplified approach described in comment #12. We could add a @noautohide panel pointing at the infobar to increase visibility of this.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8722678 - Flags: review?(jmathies)
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review32767

I have too many questions to give you r+ in one go, sorry!

::: browser/components/nsBrowserGlue.js:937
(Diff revision 1)
> +      Services.console.logStringMessage("Unknown reason given to _resetProfileNotification");

This should log the "reason" variable. Might even make more sense to throw new Error() so you get a stack.

::: browser/components/nsBrowserGlue.js:1065
(Diff revision 1)
> +      let uninstalledValue = WindowsRegistry.readRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> +                                                        "Software\\Mozilla\\Firefox",
> +                                                        "Uninstalled");

I'm fairly sure we can't rely on that key (ie the Software\\Mozilla\\Firefox bit) existing for e.g. portable versions of Firefox (or ones used from zipfiles, or...) , and that this will throw if that path doesn't exist because .open() here: https://dxr.mozilla.org/mozilla-central/rev/789a12291942763bc1e3a89f97e0b82dc1c9d00b/toolkit/modules/WindowsRegistry.jsm will throw. AFAICT the read won't throw if the path exists, but the open() call will.

More generally, I'd quite like this stuff to live in a try...catch so that if it throws unexpectedly, we don't die in startup. That's not a great reinstallation experience. :-(

::: browser/components/nsBrowserGlue.js:1068
(Diff revision 1)
> +      WindowsRegistry.removeRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> +                                   "Software\\Mozilla\\Firefox",
> +                                   "Uninstalled");

Can this fail if the user runs with restricted privileges?

It also seems like this means we'll only offer reset for the first profile you run with. That's probably not super important.

However, if you e.g. uninstall Devedition and continue using "normal" Firefox (or vice versa), you'd get this note. Is that an issue we should worry about? We could maybe write the executable path of the uninstalled thing, or the version or brand name, and only care if those match, or is that a dumb idea?

::: browser/installer/windows/nsis/uninstaller.nsi:453
(Diff revision 1)
> +  ${LogHeader} "Make note so future installs can inquire about using a fresh profile"

I'm surprised we don't delete all the registry stuff we do from the uninstaller.

If we're logging something I would prefer to be more explicit about what we're doing. We can use a comment to explain why we're doing it, and that should also mention that we'll remove the registry key as soon as Firefox first starts up.

::: browser/installer/windows/nsis/uninstaller.nsi:454
(Diff revision 1)
> +  WriteRegStr HKCU "Software\Mozilla\Firefox" "Uninstalled" "True"

Is HKCU "software\Mozilla\Firefox" guaranteed to exist here? And/or what happens when it doesn't?

::: toolkit/locales/en-US/chrome/global/resetProfile.properties:10
(Diff revision 1)
> +resetUninstalled.message=It looks like you've reinstalled %S. Do you want to clean it up for a fresh, like-new experience? And by the way, thanks for using %S!

Please get UX / messaging signoff on the language here.
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #15)
> Comment on attachment 8722678 [details]
> MozReview Request: Bug 1095739 - Allow a "new user" experience to happen
> subsequent to Firefox being uninstalled. r?gijs
> 
> https://reviewboard.mozilla.org/r/36179/#review32767
> 
> I have too many questions to give you r+ in one go, sorry!
> 
> ::: browser/components/nsBrowserGlue.js:937
> (Diff revision 1)
> > +      Services.console.logStringMessage("Unknown reason given to _resetProfileNotification");
> 
> This should log the "reason" variable. Might even make more sense to throw
> new Error() so you get a stack.

OK.

> ::: browser/components/nsBrowserGlue.js:1065
> (Diff revision 1)
> > +      let uninstalledValue = WindowsRegistry.readRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> > +                                                        "Software\\Mozilla\\Firefox",
> > +                                                        "Uninstalled");
> 
> I'm fairly sure we can't rely on that key (ie the Software\\Mozilla\\Firefox
> bit) existing for e.g. portable versions of Firefox (or ones used from
> zipfiles, or...) , and that this will throw if that path doesn't exist
> because .open() here:
> https://dxr.mozilla.org/mozilla-central/rev/
> 789a12291942763bc1e3a89f97e0b82dc1c9d00b/toolkit/modules/WindowsRegistry.jsm
> will throw. AFAICT the read won't throw if the path exists, but the open()
> call will.
> 
> More generally, I'd quite like this stuff to live in a try...catch so that
> if it throws unexpectedly, we don't die in startup. That's not a great
> reinstallation experience. :-(

WindowsRegistry.jsm wraps both the readRegKey and removeRegKey in try/catches.

> ::: browser/components/nsBrowserGlue.js:1068
> (Diff revision 1)
> > +      WindowsRegistry.removeRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> > +                                   "Software\\Mozilla\\Firefox",
> > +                                   "Uninstalled");
> 
> Can this fail if the user runs with restricted privileges?
> 
> It also seems like this means we'll only offer reset for the first profile
> you run with. That's probably not super important.
> 
> However, if you e.g. uninstall Devedition and continue using "normal"
> Firefox (or vice versa), you'd get this note. Is that an issue we should
> worry about? We could maybe write the executable path of the uninstalled
> thing, or the version or brand name, and only care if those match, or is
> that a dumb idea?

Yeah, we can include the brandShortName here.

> ::: browser/installer/windows/nsis/uninstaller.nsi:453
> (Diff revision 1)
> > +  ${LogHeader} "Make note so future installs can inquire about using a fresh profile"
> 
> I'm surprised we don't delete all the registry stuff we do from the
> uninstaller.
> 
> If we're logging something I would prefer to be more explicit about what
> we're doing. We can use a comment to explain why we're doing it, and that
> should also mention that we'll remove the registry key as soon as Firefox
> first starts up.
> 
> ::: browser/installer/windows/nsis/uninstaller.nsi:454
> (Diff revision 1)
> > +  WriteRegStr HKCU "Software\Mozilla\Firefox" "Uninstalled" "True"
> 
> Is HKCU "software\Mozilla\Firefox" guaranteed to exist here? And/or what
> happens when it doesn't?

I'm not sure. I will leave that question to :jimm.

> ::: toolkit/locales/en-US/chrome/global/resetProfile.properties:10
> (Diff revision 1)
> > +resetUninstalled.message=It looks like you've reinstalled %S. Do you want to clean it up for a fresh, like-new experience? And by the way, thanks for using %S!
> 
> Please get UX / messaging signoff on the language here.

Sure thing.
https://reviewboard.mozilla.org/r/36179/#review32767

> I'm fairly sure we can't rely on that key (ie the Software\\Mozilla\\Firefox bit) existing for e.g. portable versions of Firefox (or ones used from zipfiles, or...) , and that this will throw if that path doesn't exist because .open() here: https://dxr.mozilla.org/mozilla-central/rev/789a12291942763bc1e3a89f97e0b82dc1c9d00b/toolkit/modules/WindowsRegistry.jsm will throw. AFAICT the read won't throw if the path exists, but the open() call will.
> 
> More generally, I'd quite like this stuff to live in a try...catch so that if it throws unexpectedly, we don't die in startup. That's not a great reinstallation experience. :-(

WindowsRegistry.jsm already wraps these calls in try/catches

> Can this fail if the user runs with restricted privileges?
> 
> It also seems like this means we'll only offer reset for the first profile you run with. That's probably not super important.
> 
> However, if you e.g. uninstall Devedition and continue using "normal" Firefox (or vice versa), you'd get this note. Is that an issue we should worry about? We could maybe write the executable path of the uninstalled thing, or the version or brand name, and only care if those match, or is that a dumb idea?

Yeah, we can match brandShortName here
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review32779
https://reviewboard.mozilla.org/r/36179/#review32767

> This should log the "reason" variable. Might even make more sense to throw new Error() so you get a stack.

I don't want to throw here because this is in the startup path, but I will add the 'reason' argument to the log.
Attachment #8722678 - Attachment description: MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs → MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36179/diff/1-2/
https://reviewboard.mozilla.org/r/36179/#review32787

::: browser/components/nsBrowserGlue.js:937
(Diff revision 1)
> +      Services.console.logStringMessage("Unknown reason given to _resetProfileNotification");
> +      return;

Why not just throw? Being gentle with cases like this means they don't get caught by users or automated tests as easily.

::: browser/components/nsBrowserGlue.js:1060
(Diff revision 1)
> +      showingResetPrompt = true;
> +    }
> +
> +    // Check if we were just re-installed and offer Firefox Reset
> +    if (AppConstants.platform == "win") {

You can avoid adding `showingResetPrompt` by using:
} else if (AppConstants.platform == "win") {

As-is you technically should be setting `showingResetPrompt = true;` in the uninstall case too.

::: browser/components/nsBrowserGlue.js:1068
(Diff revision 1)
> +      WindowsRegistry.removeRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> +                                   "Software\\Mozilla\\Firefox",
> +                                   "Uninstalled");
> +      if (!disableResetPrompt && uninstalledValue == "True" &&
> +          !showingResetPrompt) {
> +        this._resetProfileNotification("uninstall");

To avoid a case where the notification appears on every startup due to not having permissions to remove the `Uninstalled` key, we could only show the notification if removeRegKey succeeded.
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review32851

::: browser/components/nsBrowserGlue.js:1063
(Diff revision 2)
> +    // Check if we were just re-installed and offer Firefox Reset
> +    if (AppConstants.platform == "win") {

concur that it would make more sense to use 'else if' here.

::: browser/components/nsBrowserGlue.js:1069
(Diff revision 2)
> +      WindowsRegistry.removeRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> +                                   "Software\\Mozilla\\Firefox",
> +                                   `Uninstalled-${brandShortName}`);

Hm, so thinking about this more... would it be more robust to compare the install time/date of the instance we're running (which presumably we know / can find out?) with the uninstall time? That would avoid both the branding issue and similar issues where the branding does match up.

::: browser/components/nsBrowserGlue.js:1069
(Diff revision 2)
> +      WindowsRegistry.removeRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> +                                   "Software\\Mozilla\\Firefox",
> +                                   `Uninstalled-${brandShortName}`);

And yes, we should only prompt if the removal succeeded. Is there some way to determine that? Seems like removeRegKey should return success or something like that...

::: browser/installer/windows/nsis/uninstaller.nsi:458
(Diff revision 2)
> +  ${LogHeader} "Make note so future installs can inquire about using a fresh profile"

For the log header I would use "Write Uninstalled-${BrandShortName} registry key", as that's more descriptive of what's actually happening.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12)
> 
> Showing the infobar after uninstall-reinstall will still be a net
> improvement and increase the utility of the code that we already have for
> doing this.

The infobar is not a great experience and doesn't solve the entire problem. If we can implement https://bugzilla.mozilla.org/attachment.cgi?id=8720448 we won't need an infobar. 


(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #13)
> Taking to implement a simplified approach described in comment #12. We could
> add a @noautohide panel pointing at the infobar to increase visibility of
> this.

I don't understand why we're doing this. Is this meant to be an interim step that we remove later?
Flags: needinfo?(jaws)
(In reply to :Gijs Kruitbosch from comment #11) 
> > 2. Unused – if the user hasn’t used Firefox for more than 60 days and
> > doesn’t have it set as the default browser, they’re not really a Firefox
> > user. So in this case we want to give them a like-new experience.
> 
> OK, so even in the 60-days-unused case you want to automatically refresh
> (rather than give them the choice but not always do it)? But only in the
> case where Firefox isn't the default browser (based on the flowchart you
> attached)?

Correct. I’m using the default browser setting as a proxy for the user declaring this as their browser of choice. So if Firefox is your browser of choice and you just don’t use the internet frequently, we don’t want to auto-refresh you all the time and then tell you why you should use Firefox. If Firefox is not your browser and you use it rarely, let's make sure you have a like-new experience and we give you some reasons to use Firefox more. 

> 
> > This often goes unnoticed/not
> > acted upon and even when used it only provides part of the solution (in this
> > case missing the ability to display a special what’s new page).
> 
> OK, so we also need a special what's new page. Note that we already have the
> 'about:welcomeback' page which lets people restore tabs and continue on
> their way. We could modify that. Opening a secondary page in addition to
> that page seems like it might not be a great experience / very useful.
> 

Yes. We’ll need to do something here. I’ll have to work on this.

> > The refresh feature was made for these purposes and
> > since we are going to migrate add-ons (disabled see Bug 1017919) there are
> > no more reasons to not do this automatically. 
> 
> Well, users' preferences don't get migrated, either...

That’s on purpose so totally fine in this case.


> 
> > I don’t think the page
> > we’ll want to display should simply be a list of new features or a
> > collection of the previous what’s new pages. It probably should also address
> > this special context.
> 
> This sounds exactly like about:welcomeback . Is it supposed to be different?

Yes, it’s supposed to be different. Also something for me to work on.

> 
> All in all this sounds like a large change that should be split off into
> several separate bugs. We also need more clarity on how the 'what's new'
> page should work, and if it should also show up if a user manually resets
> Firefox, if it's in addition to about:welcomeback or should be integrated in
> there somehow, etc.
> 
> I also think we need to fix bug 1017919 and bug 1054947 (and potentially
> other blockers of bug 498181) before enabling auto-reset in the 60-days case.
> 
> Does that make sense?

Yes, I agree that we should split this up into separate bugs and bug 1017919 is definitely a prerequisite for this.
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #23)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #13)
> > Taking to implement a simplified approach described in comment #12. We could
> > add a @noautohide panel pointing at the infobar to increase visibility of
> > this.
> 
> I don't understand why we're doing this. Is this meant to be an interim step
> that we remove later?

Yes. I think Jared's reasoning (and I would concur with him!) is that it'd be good to more explicitly point out to people that they can (and make it easier for them to) refresh Firefox, even if we're not at the stage where we can do this automatically.

This is something we can do today; as comment #24 noted, there are quite some blockers to doing this automatically.
https://reviewboard.mozilla.org/r/36179/#review32851

> Hm, so thinking about this more... would it be more robust to compare the install time/date of the instance we're running (which presumably we know / can find out?) with the uninstall time? That would avoid both the branding issue and similar issues where the branding does match up.

I'm not sure what we could infer from install time/date combined with uninstall time/date. If the app was uninstalled many moons ago, there may still be valuable information to the user in their profile or they may not have had a legitemate issue with Firefox to cause their uninstall. Or perhaps they did have issues, moved on to another browser, and are now willing to try it out again.

I think the solution already in place is sufficient for branding mismatches, though it is something to think about that Nightly and Beta/Release will use the same profile by default. Users who are using Release along with Nightly are already a small enough population to not worry about and if sharing a profile amongst the browsers may have already been running in to issues. If a user does uninstall Nightly and tries switching to Release, we will not offer them a clean start. We may want to handle that in a follow-up since there are other issues with moving a profile ~3 versions (especially backwards, Nightly -> Release).
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36179/diff/2-3/
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #26)
> https://reviewboard.mozilla.org/r/36179/#review32851
> 
> > Hm, so thinking about this more... would it be more robust to compare the install time/date of the instance we're running (which presumably we know / can find out?) with the uninstall time? That would avoid both the branding issue and similar issues where the branding does match up.
> 
> I'm not sure what we could infer from install time/date combined with
> uninstall time/date. If the app was uninstalled many moons ago, there may
> still be valuable information to the user in their profile or they may not
> have had a legitemate issue with Firefox to cause their uninstall. Or
> perhaps they did have issues, moved on to another browser, and are now
> willing to try it out again.
> 
> I think the solution already in place is sufficient for branding mismatches,
> though it is something to think about that Nightly and Beta/Release will use
> the same profile by default. Users who are using Release along with Nightly
> are already a small enough population to not worry about and if sharing a
> profile amongst the browsers may have already been running in to issues. If
> a user does uninstall Nightly and tries switching to Release, we will not
> offer them a clean start.

Or vice versa. Another reason, maybe, to use timestamps rather than branding?

> We may want to handle that in a follow-up since
> there are other issues with moving a profile ~3 versions (especially
> backwards, Nightly -> Release).

I'm trying to figure out how to ignore the case where uninstall time is *later* than install time of the running instance, as it would be if you had multiple apps installed and uninstalled one of them, ie to detect the case where you have not, in fact, installed a copy of Firefox (of whichever version) since uninstalling at all. Does that help clarify why the timestamp might be useful?
That is, I'm explicitly not trying to suggest that, if you reinstall Firefox after uninstalling it, the time that has elapsed since uninstall should make a difference as to whether we show this infobar. Just that if you "start with" 2 copies, uninstall 1, then run the other copy, this infobar should not be triggered. That gets more important if we later change this code to do the migration/refresh automatically.
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review33009

Clearing review because of the install time stuff.

::: toolkit/modules/WindowsRegistry.jsm:60
(Diff revision 3)
> +   * @return True if the key was removed, false otherwise.

Nit: true ... or if the key never existed, false otherwise

right?
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
Note-to-self, we can use http://mxr.mozilla.org/mozilla-central/search?string=gettime&find=installer&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central for retrieving the date/time and storing that in the registry.
Flags: needinfo?(jaws)
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review33239

::: browser/components/nsBrowserGlue.js:1063
(Diff revision 3)
> +        WindowsRegistry.readRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,

We shouldn't have access issues here as long as this is in HKCU.

::: browser/installer/windows/nsis/uninstaller.nsi:457
(Diff revision 3)
> +  ; uninstalls of Firefox/DevEdition with reinstalls of Firefox/DevEdition, respectively.

http://mxr.mozilla.org/mozilla-central/find?text=&string=branding.nsi

Looking at our use of BrandShortName it looks like this won't pick up differences between beta/release and possible esr. Personally I do not care as much as some about the multiple installs case. But since we plan on doing this refresh automatically at some point I think we need to switch to something else. I would probably trigger this unknowningly in my beta since I rarely run it.
Attachment #8722678 - Flags: review?(jmathies)
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36179/diff/3-4/
Attachment #8722678 - Flags: review?(jmathies)
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
I've switched from using the brandShortName to using the updateChannel, as that will be different for all releases of Firefox (local builds will use "default").
https://reviewboard.mozilla.org/r/36179/#review45863

::: browser/components/nsBrowserGlue.js:1054
(Diff revision 4)
>          Date.now() - lastUse >= OFFER_PROFILE_RESET_INTERVAL_MS) {
> -      this._resetUnusedProfileNotification();
> +      this._resetProfileNotification("unused");
> +    } else if (AppConstants.platform == "win") {
> +      // Check if we were just re-installed and offer Firefox Reset
> +      let updateChannel;
> +      Services.console.logStringMessage("jaws: firefox reinstalled path");

Note that you left a lot of logging in this file with your name.
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36179/diff/4-5/
https://reviewboard.mozilla.org/r/36179/#review45865

::: browser/components/nsBrowserGlue.js:15
(Diff revisions 3 - 5)
> +// Set us up to use async prefs in the parent process.
> +Cu.import("resource://gre/modules/AsyncPrefs.jsm");

These changes somehow snuck in... I'm looking in to it
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #37)
> https://reviewboard.mozilla.org/r/36179/#review45865
> 
> ::: browser/components/nsBrowserGlue.js:15
> (Diff revisions 3 - 5)
> > +// Set us up to use async prefs in the parent process.
> > +Cu.import("resource://gre/modules/AsyncPrefs.jsm");
> 
> These changes somehow snuck in... I'm looking in to it

Interdiff is just lying to you because you rebased. Those changes aren't in v5 of the diff.
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review46049

r=me assuming r+ from jimm and with the below addressed.

::: browser/components/nsBrowserGlue.js:1066
(Diff revision 5)
> +                                     `Uninstalled-${updateChannel}`);
> +        let removalSuccessful =
> +          WindowsRegistry.removeRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> +                                       "Software\\Mozilla\\Firefox",
> +                                       `Uninstalled-${updateChannel}`);
> +        if (!disableResetPrompt && removalSuccessful &&

Nit: feels like the check for `!disableResetPrompt` should be in the `} else if (AppConstants.platform == "win") {` condition instead.

::: browser/installer/windows/nsis/uninstaller.nsi:453
(Diff revision 5)
>    ; Refresh desktop icons otherwise the start menu internet item won't be
>    ; removed and other ugly things will happen like recreation of the app's
>    ; clients registry key by the OS under some conditions.
>    System::Call "shell32::SHChangeNotify(i ${SHCNE_ASSOCCHANGED}, i 0, i 0, i 0)"
>  
> +  ; Users who uninstall then reinstall expecting Firefox to use a clean profile

Is this going to run for pave-over installs? So if I don't run the uninstaller first but run the installer directly?

On which note, would it make more sense to just set this in the *installer* iff it detects that profiles exist? I guess that could be a followup if this doesn't already work in that case.

::: toolkit/modules/WindowsRegistry.jsm:79
(Diff revision 5)
>          registry.removeValue(aKey);
> +        result = !registry.hasValue(aKey);

If `removeValue` throws with e.g. permission denied, result will still be `true`, which seems like it'd be wrong?
Attachment #8722678 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review46059

Hrmpf, forgot to check ship-it. Sorry for the spam.
Attachment #8722678 - Flags: review+
Comment on attachment 8722678 [details]
MozReview Request: Bug 1095739 - Allow a "new user" experience to happen subsequent to Firefox being uninstalled. r?gijs,jimm

https://reviewboard.mozilla.org/r/36179/#review46075

::: toolkit/locales/en-US/chrome/global/resetProfile.properties:10
(Diff revision 5)
>  # LOCALIZATION NOTE: These strings are used for profile reset.
>  
>  # LOCALIZATION NOTE (resetUnusedProfile.message): %S is brandShortName.
>  resetUnusedProfile.message=It looks like you haven't started %S in a while. Do you want to clean it up for a fresh, like-new experience? And by the way, welcome back!
> +# LOCALIZATION NOTE (resetUninstalled.message): %S is brandShortName.
> +resetUninstalled.message=It looks like you've reinstalled %S. Do you want to clean it up for a fresh, like-new experience? And by the way, thanks for using %S!

Someone in UX should approve these strings. I'd suggest something shorter like: 

"It looks like you've reinstalled Firefox. Would you like a fresh, like-new experience?"

and I'm not sure about this:

"And by the way, thanks for using Firefox!"

I guesss it depends on how much room we have where this is displayed. In languages like Russian this could end up being a really long string.

::: toolkit/modules/WindowsRegistry.jsm:72
(Diff revision 5)
> +   * @return True if the key was removed or never existed, false otherwise.
>     */
>    removeRegKey: function(aRoot, aPath, aKey, aRegistryNode=0) {
>      let registry = Cc["@mozilla.org/windows-registry-key;1"].
>                     createInstance(Ci.nsIWindowsRegKey);
> +    let result = true;

Yeah I think this should default to false since your comment indicates "everything else" should return false.
Attachment #8722678 - Flags: review?(jmathies) → review+
https://reviewboard.mozilla.org/r/36179/#review46075

> Someone in UX should approve these strings. I'd suggest something shorter like: 
> 
> "It looks like you've reinstalled Firefox. Would you like a fresh, like-new experience?"
> 
> and I'm not sure about this:
> 
> "And by the way, thanks for using Firefox!"
> 
> I guesss it depends on how much room we have where this is displayed. In languages like Russian this could end up being a really long string.

I asked for feedback in #ux and got the following string from helenvholmes,
resetUninstalled.message=Looks like you've reinstalled %S. Do you want to clean it up for a fresh, like-new experience?
Correction, the final string is:
resetUninstalled.message=Looks like you've reinstalled %S. Want us to clean it up for a fresh, like-new experience?
Attachment #8722678 - Attachment is obsolete: true
Flags: needinfo?(mconnor)
Attachment #8746125 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/2157fe6b6efd
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Is there a 'kill' pref hidden, or otherwise for this feature. 

Nightly testers often uninstall/re-install during testing for regressions and getting the 'Notice' to reset is now an unneeded feature for dedicated Nightly testers.

This should be maybe for 'Release' only, not Nightly/ Maybe Aurora ?
Depends on: 1270089
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #47)
> Is there a 'kill' pref hidden, or otherwise for this feature. 
> 
> Nightly testers often uninstall/re-install during testing for regressions
> and getting the 'Notice' to reset is now an unneeded feature for dedicated
> Nightly testers.
> 
> This should be maybe for 'Release' only, not Nightly/ Maybe Aurora ?

Thanks, I filed bug 1270089 and will have a patch up there today to disable the feature if a pref in about:config is enabled. I don't think this feature should be for Release only, as users on other channels may be experiencing profile-related issues and blame the default install of Firefox for the reasons that are otherwise caused by some configuration in their profile.
Depends on: 1554267
Depends on: 1290500
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: