1448804 - Point users to a feedback form at the end of the uninstall process

Status: RESOLVED FIXED

(Firefox :: Installer, defect, P1)

Description

[Romain Testard [:RT]]

Point users to an uninstall form at the end of the uninstallation process.

Comment 2

[Romain Testard [:RT]]

Rob, per discussion on bug 1451279, a URL from qsurvey.mozilla.org will be fine for our needs here, can you please help provide one for us to use in the client?

Comment 3

[Tyler Downer [He/Him]]

Romain,
 We will want to work to design the survey soon (as this URL points to an empty survey), but here is a URL:

https://qsurvey.mozilla.com/s3/FF-Desktop-Post-Uninstall

You can pass through whatever URL variables you want, we will be able to capture them on the survey side.

Comment 4

[Romain Testard [:RT]]

I'm updating the user story to set the share of uninstall users exposed to the survey to 10%. This would be using a mechanism where the uninstaller picks a random number based on a high-resolution timer or a similar approach, and picks the 10% sample from that. 
Doing this through Shield does not sound appropriate given this potentially would have to remain available for ever and Shield is designed to managed surveys for a definite duration.
We can still adjust the 10% value later based on what we learn when this ships, although understanding that only new installs would see the 10% value change.

Comment 5

[Molly Howell (she/her) (no longer active)]

(In reply to Romain Testard [:RT] from comment #4)
> We can still adjust the 10% value later based on what we learn when this
> ships, although understanding that only new installs would see the 10% value
> change.

We would deploy the changed value through app updates as well, not only for new installs.

Comment 6

[Tyler Downer [He/Him]]

What number of users uninstall Firefox per-day? If we are only displaying to 10% of users, and then this requires opting-in AND the user to actually fill out the survey, I think the funnel will have such a massive drop-off that we will see few users actually take this survey. Compounding that, my theory is that few users who stop using Firefox actually uninstall it, but just install a new browser and switch over.

I'm beginning to wonder about the usefulness of this at this point.

Comment 7

[Romain Testard [:RT]]

(In reply to Tyler Downer [:Tyler] from comment #6)
> What number of users uninstall Firefox per-day?
We don't have that data currently

 If we are only displaying to
> 10% of users, and then this requires opting-in
This is opt-out, this is now clarified in the user story.

 AND the user to actually fill
> out the survey, I think the funnel will have such a massive drop-off that we
> will see few users actually take this survey. Compounding that, my theory is
> that few users who stop using Firefox actually uninstall it, but just
> install a new browser and switch over.
> 
> I'm beginning to wonder about the usefulness of this at this point.
It's hard to assess how many uninstalls we get. I agree that starting at 10% may require increasing that throttle if we realize volume is too low. We could start at 100% and then decrease if volume is too high obviously. As Matt mentioned the throttle value can be adjusted through app update. 
I'll reach out to discuss and get to an agreement on a good share of users to expose to the survey.

Comment 8

[Romain Testard [:RT]]

> (In reply to Tyler Downer [:Tyler] from comment #6)
> > What number of users uninstall Firefox per-day?
> We don't have that data currently
> 
>  If we are only displaying to
> > 10% of users, and then this requires opting-in
> This is opt-out, this is now clarified in the user story.
> 
Correcting myself, this is opt-in per chat with legal - user story corrected

Comment 9

[Romain Testard [:RT]]

Given that this survey has to be opt-in we won't be able to reliably quantify uninstalls. 
Bug 1461690 will help quantify uninstalls (telemetry uninstall ping) and we'll still ship this survey ahead of bug 1461690. Bug 1461690 will help understand statistical significance of the feedback gathered as well as other benefits outlined on the bug.

Comment 10

[Romain Testard [:RT]]

Michelle, can you please help validate the string we should be using for the opt-in checkbox in the uninstaller?
Proposal: I want to give details to Mozilla about why I just uninstalled Firefox.

Comment 11

[Romain Testard [:RT]]

US updated with latest details.
The opt-in checkbox should be exposed to all users given this is only opt-in so it helps us get a reasonable number of answers.
Mika, could you please review that the survey is OK from a legal standpoint?
https://qsurvey.mozilla.com/s3/FF-Desktop-Post-Uninstall

Comment 12

[Romain Testard [:RT]]

(In reply to Romain Testard [:RT] from comment #10)
> Michelle, can you please help validate the string we should be using for the
> opt-in checkbox in the uninstaller?
> Proposal: I want to give details to Mozilla about why I just uninstalled
> Firefox.

Hi Michelle, we want to ship this with 63, can you please help confirm the string is correct?
This will appear on the Windows uninstaller on the Finish page along with an opt-in check-box to enter a survey (Web-based on https://qsurvey.mozilla.com/s3/FF-Desktop-Post-Uninstall)

Comment 13

[Romain Testard [:RT]]

Clearing NI to Mika since this was discussed separately through e-mail.

Comment 14

[Molly Howell (she/her) (no longer active)]

Attachment: Bug 1448804 Part 1 - NSIS plugin for creating unelevated processes from an elevated [un]installer.

Review commit: https://reviewboard.mozilla.org/r/255824/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255824/

Comment 15

[Molly Howell (she/her) (no longer active)]

Attachment: Bug 1448804 Part 2 - Offer an opt-in survey at the end of the Windows uninstaller.

Review commit: https://reviewboard.mozilla.org/r/255826/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255826/

Comment 16

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8990774 [details]
Bug 1448804 Part 2 - Offer an opt-in survey at the end of the Windows uninstaller.

https://reviewboard.mozilla.org/r/255826/#review262608

::: browser/locales/en-US/installer/custom.properties:71
(Diff revision 1)
>  STATUS_INSTALL_APP=Installing $BrandShortName…
>  STATUS_INSTALL_LANG=Installing Language Files (${AB_CD})…
>  STATUS_UNINSTALL_MAIN=Uninstalling $BrandShortName…
>  STATUS_CLEANUP=A Little Housekeeping…
>  
> +UN_SURVEY_CHECKBOX_LABEL=I want to give details to Mozilla about why I just uninstalled Firefox.

This may or may not be the final string, I'll wait to land the patch until that's confirmed.

Comment 17

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8990773 [details]
Bug 1448804 Part 1 - NSIS plugin for creating unelevated processes from an elevated [un]installer.

https://reviewboard.mozilla.org/r/255824/#review262654

I'd like to see one more iteration of this.

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:34
(Diff revision 1)
> +  IShellView* view = nullptr;
> +  IShellWindows* shell = nullptr;
> +  CoCreateInstance(CLSID_ShellWindows, NULL, CLSCTX_LOCAL_SERVER,
> +                   IID_PPV_ARGS(&shell));
> +  if (shell) {
> +    VARIANT empty = {};

I'd prefer using VariantInit() here

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:37
(Diff revision 1)
> +                   IID_PPV_ARGS(&shell));
> +  if (shell) {
> +    VARIANT empty = {};
> +    HWND windowHandle = 0;
> +    IDispatch* dispatch = nullptr;
> +    shell->FindWindowSW(&empty, &empty, SWC_DESKTOP, (long*)&windowHandle,

The first parameter is supposed to be CSIDL_DESKTOP

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:41
(Diff revision 1)
> +    IDispatch* dispatch = nullptr;
> +    shell->FindWindowSW(&empty, &empty, SWC_DESKTOP, (long*)&windowHandle,
> +                        SWFO_NEEDDISPATCH, &dispatch);
> +    if (dispatch) {
> +      IServiceProvider* provider = nullptr;
> +      dispatch->QueryInterface(IID_IServiceProvider, (void**)&provider);

I guess you could use IID_PPV_ARGS here for consistency

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:63
(Diff revision 1)
> +
> +static IShellDispatch2*
> +GetApplicationFromShellView(IShellView* view)
> +{
> +  IShellDispatch2* shellDispatch = nullptr;
> +  HRESULT hr = view->GetItemObject(SVGIO_BACKGROUND, IID_IDispatch,

This looks like a leak. Please use a different pointer for the outparam.

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:83
(Diff revision 1)
> +  return shellDispatch;
> +}
> +
> +static bool
> +ShellExecInExplorerProcess(wchar_t* path)
> +{

As per our discussion on IRC, I'd suggest that you wrap this with CoInitialize/CoUninitialize to ensure that we have COM; it'll just bump a refcount if NSIS has already done so.

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:87
(Diff revision 1)
> +ShellExecInExplorerProcess(wchar_t* path)
> +{
> +  bool rv = false;
> +  IShellView *desktopView = GetDesktopWindowShellView();
> +  if (desktopView)
> +  {

Nit: indentation style

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:148
(Diff revision 1)
> +  stack_t *th;
> +  if (!stacktop) {
> +    return;
> +  }
> +
> +  th = (stack_t*)GlobalAlloc(GPTR, sizeof(stack_t) + len);

Casting directly from HGLOBAL to other types (without jumping through GlobalLock) makes me queasy. Is this a common NSIS-ism?

Comment 18

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8990773 [details]
Bug 1448804 Part 1 - NSIS plugin for creating unelevated processes from an elevated [un]installer.

https://reviewboard.mozilla.org/r/255824/#review262654

> Casting directly from HGLOBAL to other types (without jumping through GlobalLock) makes me queasy. Is this a common NSIS-ism?

It is. It's probably not the only common NSIS-ism that would make you queasy, along with any other reasonable person. It's easy enough to just change this to HeapAlloc though.

Comment 19

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8990773 [details]
Bug 1448804 Part 1 - NSIS plugin for creating unelevated processes from an elevated [un]installer.

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

Comment 20

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8990774 [details]
Bug 1448804 Part 2 - Offer an opt-in survey at the end of the Windows uninstaller.

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

Comment 21

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Comment on attachment 8990774 [details]
Bug 1448804 Part 2 - Offer an opt-in survey at the end of the Windows uninstaller.

https://reviewboard.mozilla.org/r/255826/#review263130

Looks good once the string is finalized (and parameterized)!

Comment 22

[mheubusch]

Can you change the initial sentence to Firefox has been uninstalled. Can you tell us why you left?

Otherwise it's good to go.

Comment 23

[Molly Howell (she/her) (no longer active)]

Attachment: Finish Page w/ checkbox

Just to confirm, the screen does already say "Firefox has been uninstalled from your computer.", so does the checkbox label need to restate that? I'll attach a screenshot to illustrate what that screen looks like; for some reason I thought there already was one around somewhere, sorry. The checkbox (and label) near the center is the only thing we're adding here, the rest is what's there already.

Comment 24

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8990773 [details]
Bug 1448804 Part 1 - NSIS plugin for creating unelevated processes from an elevated [un]installer.

https://reviewboard.mozilla.org/r/255824/#review263176

r=me with the issue fixed.

::: other-licenses/nsis/Contrib/ExecInExplorer/ExecInExplorer.cpp:72
(Diff revisions 1 - 2)
>  
>  static IShellDispatch2*
>  GetApplicationFromShellView(IShellView* view)
>  {
>    IShellDispatch2* shellDispatch = nullptr;
> -  HRESULT hr = view->GetItemObject(SVGIO_BACKGROUND, IID_IDispatch,
> +  IDispatch* viewDisp = nullptr;

I think you still need to release viewDisp when you're done with it.

Comment 25

[mheubusch]

Comment on attachment 8991384 [details]
Finish Page w/ checkbox

Oh - I looked at the wrong attachment (the websurvey itself). Two suggestions:

1. Move the checkbox above the instruction to select Finish to close the wizard, so users at least see it before they select finish.  
2. Change the wording to "Tell Mozilla why you uninstalled Firefox" Note there is no period at the end of the checkbox label
I'm also wondering if you can make that a link instead of a check box - that will signal that they are going to a webpage. (Not sure if that's feasible).

Comment 26

[Molly Howell (she/her) (no longer active)]

(In reply to mheubusch from comment #25)
> Comment on attachment 8991384 [details]
> Finish Page w/ checkbox
> 
> Oh - I looked at the wrong attachment (the websurvey itself). Two
> suggestions:
> 
> 1. Move the checkbox above the instruction to select Finish to close the
> wizard, so users at least see it before they select finish.  
> 2. Change the wording to "Tell Mozilla why you uninstalled Firefox" Note
> there is no period at the end of the checkbox label
> I'm also wondering if you can make that a link instead of a check box - that
> will signal that they are going to a webpage. (Not sure if that's feasible).

Okay, thanks. I can't easily move the checkbox, so I'll go with option 2. A link would be preferable, but unfortunately it doesn't work because of the hoops we have to jump through to start the user's browser in a secure way.

Comment 27

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8990773 [details]
Bug 1448804 Part 1 - NSIS plugin for creating unelevated processes from an elevated [un]installer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255824/diff/2-3/

Comment 28

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8990774 [details]
Bug 1448804 Part 2 - Offer an opt-in survey at the end of the Windows uninstaller.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255826/diff/2-3/

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0987d300d69e
Part 1 - NSIS plugin for creating unelevated processes from an elevated [un]installer. r=aklotz
https://hg.mozilla.org/integration/autoland/rev/c2c011ae3954
Part 2 - Offer an opt-in survey at the end of the Windows uninstaller. r=agashlin

Comment 30

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/0987d300d69e
https://hg.mozilla.org/mozilla-central/rev/c2c011ae3954