The uninstall survey is not opened when Firefox was the default browser

VERIFIED FIXED in Firefox 63

Status

()

defect
P1
normal
VERIFIED FIXED
10 months ago
10 months ago

People

(Reporter: cbaica, Assigned: mhowell)

Tracking

63 Branch
Firefox 64
Desktop
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63+ verified, firefox64 verified)

Details

Attachments

(4 attachments, 2 obsolete attachments)

Posted video uninstall survey bug
[Affected versions]:
- Fx63.0b9

[Affected platforms]:
- Window 7 x64

[Steps to reproduce]:
1. Run the Firefox installer, making sure the option to make Firefox the default browser is checked.
2. Uninstall Firefox from the control panel.
3. After the uninstall is finished, check the option to "Tell Mozilla why you uninstalled Firefox".

[Expected result]:
- The user should have the uninstall survey web-page displayed in another browser available on the system (e.g. IE).

[Actual result]:
- An error is received informing the user that there is no program associated with the type of file he wants to open.

[Regression range]:
- This is not a new regression since it's related to a new feature.

[Additional notes]:
- In the "Default Programs" section, IE appears as the default program to open .html files.
- This seems to be isolated to my machine, because on another try on a different PC(with windows 7 setup), the issue couldn't be reproduced.
- The issue is not reproducible on windows 8.1 or 10.
Tracking for 63 as per the Beta Preliminary status report from QA which identified fixing and uplifting this bug as important for the readiness of the feature for release.
Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Priority: -- → P1
I've not been able to reproduce this at all, but I think I can fix it by forcing the page to open in Internet Explorer instead of trying to use the default browser setting.
> +    if (args[0] != L'\0') {
> +      pushstring(stacktop, args, lstrlenW(args));
> +    }
Is it not possible for there to have been an empty string on the stack?

> +      ExecInExplorer::Exec "C:\Program Files\Internet Explorer\iexplore.exe" /cmdargs "$R1"
I'm not sure how this works with localization, is this path going to work on all languages and reasonable versions of IE?
(In reply to Adam Gashlin [:agashlin] from comment #6)
> > +    if (args[0] != L'\0') {
> > +      pushstring(stacktop, args, lstrlenW(args));
> > +    }
> Is it not possible for there to have been an empty string on the stack?

Yeah, I didn't think of that. This should check for the stack itself actually being empty.

> > +      ExecInExplorer::Exec "C:\Program Files\Internet Explorer\iexplore.exe" /cmdargs "$R1"
> I'm not sure how this works with localization, is this path going to work on
> all languages and reasonable versions of IE?

Oh, geez, yeah. At the very least I shouldn't have hard coded "Program Files". I'll look into this.
Improved the stack restoration logic.
Attachment #9012902 - Attachment is obsolete: true
Attachment #9012902 - Flags: review?(agashlin)
Attachment #9012981 - Flags: review?(agashlin)
Turns out you don't even need the path to iexplore.exe, presumably because it's registered in App Paths.
Attachment #9012903 - Attachment is obsolete: true
Attachment #9012903 - Flags: review?(agashlin)
Attachment #9012984 - Flags: review?(agashlin)
Attachment #9012981 - Flags: review?(agashlin) → review+
Attachment #9012984 - Flags: review?(agashlin) → review+
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6006b0096da2
Part 1 - Support optional command line arguments in the ExecInExplorer plugin. r=agashlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8d8c5328915
Part 2 - Always use IE to show the uninstall survey on Win7/8. r=agashlin
https://hg.mozilla.org/mozilla-central/rev/6006b0096da2
https://hg.mozilla.org/mozilla-central/rev/b8d8c5328915
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Tested on the latest Fx 64.0a1. The survey is opened using IE.

Waiting for the uplift to beta in order to close the ticket.
Depends on: 1495212
We've got a regression that this patch created (bug 1495212); I need to land a fix for that before anything can get uplifted.
Comment on attachment 9012981 [details] [diff] [review]
Part 1 - Support optional command line arguments in the ExecInExplorer plugin

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1448804

User impact if declined: Uninstall survey may not open on Windows 7 and 8 (Windows 10 isn't affected).

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1495212

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This patch has been verified by QA (see comment 12).
Also, the affected feature is low impact and self-contained and the code runs after the end of the uninstallation, so the uninstall process itself is not impacted.

String changes made/needed:
Attachment #9012981 - Flags: approval-mozilla-beta?
Comment on attachment 9012981 [details] [diff] [review]
Part 1 - Support optional command line arguments in the ExecInExplorer plugin

Tracked bug for 63, approved for 63.0b13.
Attachment #9012981 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed on latest beta (Fx63.0b13) and latest nightly. 
The uninstall survey is now correctly opened in IE after having Firefox set as the default browser. If another browser is set as default (e.g. Chrome) the uninstall survey page will still open in IE as per implementation.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.