798166 - Intermittent defaults display problem after setting default depending on helper.exe speed

Status: RESOLVED FIXED

(Firefox :: Shell Integration, defect)

Description

[Brian R. Bondy [:bbondy]]

After clicking set default browser in preferences < win8, it will sometimes not update the UI to indicate you have the default browser.

This change:
-    document.getElementById("setDefaultPane").selectedIndex = 1;
+    let selectedIndex =
+      shellSvc.isDefaultBrowser(false, true) ? 1 : 0;
+    document.getElementById("setDefaultPane").selectedIndex = selectedIndex;

Caused an intermittent display problem on Windows when setting the default browser.
The real problem is inside windows shell code, where we return success before the launched helper.exe finishes.
But I don't think introducing a wait for helper.exe process is a good thing to do.

The best way to fix this is to just enable the timer always, not just on Windows 8.
The user can go into control panel while the window is open anyway and set defaults that way.

Comment 1

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1

Comment 2

[Brian R. Bondy [:bbondy]]

We'll request to get this in v17 once it is beta, but not v16.

Comment 3

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2

Just synced the 2 files comments since the last patch

Comment 4

[Brian R. Bondy [:bbondy]]

I tested this manually and pushed to try with a different patch and it all passes btw if that helps when reviewing it.

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 668271 [details] [diff] [review]
Patch v2

Review of attachment 668271 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/preferences/in-content/advanced.js
@@ +34,1 @@
>      if (!shellSvc.isDefaultBrowser(false, true)) {

nit discussed on irc: we'll remove this condition and always set up the timer because the user might change this through another channel while the pref window is open

Comment 6

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2091f0330efd

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2091f0330efd

Comment 8

[Brian R. Bondy [:bbondy]]

Comment on attachment 668271 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 791019
User impact if declined: In preferences, after clicking set default browser on Win7 and below, it will intermittently lead to the button not showing that you are the default.  The user would think they didn't set the default, but it would actually be set.  It is more of a polish regression.  It looks bad that the UI doesn't react as the user expects.
Testing completed (on m-c, etc.): I tested on m-c in Win7 and Win8 and it was working correctly.
Risk to taking this patch (and alternatives if risky): Very low 
String or UUID changes made by this patch: None

Comment 9

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2a8dc99956f6
https://hg.mozilla.org/releases/mozilla-aurora/rev/df05ca8e466e