[Tracking Requested - why for this release]: We need to get this uplifted for Firefox 40 and remove the A/B test before going to release.

Benjamin, do we need to do anything special to remove a telemetry probe?

No

Comment on attachment 8633514 [details] [diff] [review] Patch Review of attachment 8633514 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/shell/nsWindowsShellService.cpp @@ -768,5 @@ > // fall back to showing control panel for all defaults > if (NS_FAILED(rv)) { > if (IsWin10OrLater()) { > - rv = LaunchModernSettingsDialogDefaultApps(); > - } else { |if (IsWin10OrLater())| should be changed to |if (!IsWin10OrLater())|.

(In reply to Masatoshi Kimura [:emk] from comment #3) > Comment on attachment 8633514 [details] [diff] [review] > Patch > > Review of attachment 8633514 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/components/shell/nsWindowsShellService.cpp > @@ -768,5 @@ > > // fall back to showing control panel for all defaults > > if (NS_FAILED(rv)) { > > if (IsWin10OrLater()) { > > - rv = LaunchModernSettingsDialogDefaultApps(); > > - } else { > > |if (IsWin10OrLater())| should be changed to |if (!IsWin10OrLater())|. Actually, the control panel should work for Windows 10. I'll just remove the conditional around that call. Thanks for catching that.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4) > Actually, the control panel should work for Windows 10. I'll just remove the > conditional around that call. Thanks for catching that. The control panel worked before bug 791501, but it no longer works because Windows 10 blocks IAARUI::LaunchAdvancedAssociationUI. So you can't remove the version check. (Or do you expect this dialog [1]? This is in the unlikely fallback branch anyway.) [1] https://bug1167294.bugzilla.mozilla.org/attachment.cgi?id=8608889

Ok, I was unaware of bug 791501. Since this code path is only hit when LaunchModernSettingsDialogDefaultApps() fails, I think it is fine to show the warning bar/dialog since the user may have to manually make the change themselves (or find out that their account lacks the permissions to do so).

Comment on attachment 8633528 [details] [diff] [review] Patch v1.1 Review of attachment 8633528 [details] [diff] [review]: ----------------------------------------------------------------- rs=me ::: browser/components/nsBrowserGlue.js @@ -166,5 @@ > XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager", > "resource://gre/modules/LightweightThemeManager.jsm"); > > -XPCOMUtils.defineLazyModuleGetter(this, "AppConstants", > - "resource://gre/modules/AppConstants.jsm"); Kinda sad we're ditching this, I'm likely to still want to use this in this file, but adding it back is almost free, so let's ditch it for now.

The A/B test wasn't uplifted to 40, removing tracking.

Comment on attachment 8633528 [details] [diff] [review] Patch v1.1 Approval Request Comment [Feature/regressing bug #]: bug 1176112 [User impact if declined]: different default-browser experience for users given at random [Describe test coverage new/current, TreeHerder]: manual testing, landed on mozilla-central [Risks and why]: none expected [String/UUID change made/needed]: none

Comment on attachment 8633528 [details] [diff] [review] Patch v1.1 This patch seems safe and low risk given that it is turning off some settings. Let's land it on Aurora.