Brian R. Bondy [:bbondy] Assignee
	Description • 11 years ago

Currently some users have manual updates specified in Desktop Firefox options.
But we use a different profile for Metro Firefox so that option is not respected.

Adding an option in Metro preferences will not fix this because both Metro and Desktop share the same installation directory and files.

That means that some users who are expecting manual updates will get auto updated when bug 794937 lands and when the Metro browser starts to be enabled past Nightly.

I think we should take the manual update setting out of the prefs and move it into HKCU.  The HKCU would be initialized with whatever value the pref currently has, and then the pref wouldn't be used after that original initialization of the HKCU value.

I don't think this should hold up the story in bug 833182 but I think that we should create a new story or just use this as a defect on bug 833182.

	Brian R. Bondy [:bbondy] Assignee
	Comment 1 • 11 years ago

Thoughts on Comment 0? It would avoid any new UX decisions.

	Brian R. Bondy [:bbondy] Assignee
	Comment 2 • 11 years ago

As it stands now i a user has manual updates, the Metro browser will get auto updated but the Desktop browser will be manual.

	Asa Dotzler [:asa]
	Comment 3 • 11 years ago

(In reply to Brian R. Bondy [:bbondy] from comment #2)
> As it stands now i a user has manual updates, the Metro browser will get
> auto updated but the Desktop browser will be manual.

I'm unclear what the results of the proposed change would be.  Are we splitting the desktop pref out so that desktop and metro have different settings or are we making the desktop setting propagate to metro?

	Brian R. Bondy [:bbondy] Assignee
	Comment 4 • 11 years ago

Currently we have a Desktop pref that applies to Desktop (Accessible through options).

And we have a Metro pref that applies to Metro (Only accessible through about:config).

I'm suggesting that we make the option in Firefox Desktop Options modify the registry instead of the profile so that both Metro an Desktop would share this same value.  We can add an option into Firefox Metro Options if we want to or just let the user change it in the desktop browser.  I think just letting desktop handle it is sufficient. 

The problem is that unlike other prefs, this one controls the updates of the installation directory which is shared by both browsers.  So if a user thinks they disabled updates, they'd still get the update of Desktop Firefox by running Metro.

	Brian R. Bondy [:bbondy] Assignee
	Comment 5 • 11 years ago

We could also perhaps introduce a shared prefs location and attempt to migrate peoples existing settings to that shared prefs file.  Registry seems like the easiest option to me though.  This is a unique problem to Windows + Metro.

	Asa Dotzler [:asa]
	Comment 6 • 11 years ago

(In reply to Brian R. Bondy [:bbondy] from comment #4)
> Currently we have a Desktop pref that applies to Desktop (Accessible through
> options).
> 
> And we have a Metro pref that applies to Metro (Only accessible through
> about:config).
> 
> I'm suggesting that we make the option in Firefox Desktop Options modify the
> registry instead of the profile so that both Metro an Desktop would share
> this same value.  We can add an option into Firefox Metro Options if we want
> to or just let the user change it in the desktop browser.  I think just
> letting desktop handle it is sufficient. 
> 
> The problem is that unlike other prefs, this one controls the updates of the
> installation directory which is shared by both browsers.  So if a user
> thinks they disabled updates, they'd still get the update of Desktop Firefox
> by running Metro.

OK. Thanks for the clarification. This sounds good to me. Let's make desktop the only interface and respect its setting in both browsers through the registry.

	Brian R. Bondy [:bbondy] Assignee
	Comment 7 • 11 years ago

p=2 if rstrong agrees with the approach.

	Robert Strong (they/them - no direct email)
	Comment 8 • 11 years ago

The pref has to be used to allow administrators to lock down the preference.

	Robert Strong (they/them - no direct email)
	Comment 9 • 11 years ago

I think a solution based on our pref system is going to be needed because of comment #8 and that I don't think we want to go down the path of having a one-off just for this.

	Brian R. Bondy [:bbondy] Assignee
	Comment 10 • 11 years ago

rstrong: I'm wondering where we'd store it to apply to both (or all?) profiles. Would you be against just using the pref on desktop and then also replicate it into the registry for read only access from Metro?  I can build it in such a way that we can use this same method for any read-only prefs on the Metro side.

	Robert Strong (they/them - no direct email)
	Comment 11 • 11 years ago

I was thinking that might be the simplest way though there may be things I haven't thought about yet that might make that problematic.

	Brian R. Bondy [:bbondy] Assignee
	Comment 12 • 11 years ago

OK thanks for the input, I'll likely go with a solution like that but will take care to make sure we're safe about not reaching a situation where registry != pref.

I'll just make this block the story in bug 833182 since that is carrying into it6 anyway.

	Brian R. Bondy [:bbondy] Assignee
	Comment 13 • 11 years ago

So here's the deal with the patch:
1. Desktop Firefox, after startup is done, will copy an array of prefs to the registry (overwriting old values if they are already there).
2. Metro Firefox, after startup is done, will apply the prefs in the array to the current profile.
3. Desktop Firefox will sync up prefs to the registry as soon as they are changed.
4. There is no observer for registry changes, we have some support for detecting changed values in nsIWindowsRegKey, but the problem with the current nsIWindowsRegKey implementation is we'd have to have a timer in our front end code that checks every X minutes if changes exist.  That would be kind of ugly. 
5. All of this is extensible just by adding a single pref name to the array described in point #1.

	Jim Mathies [:jimm]
	Comment 14 • 11 years ago

Looks like all desktop installs would write to these keys. Maybe we should only write these out in desktop if the desktop browser is set as the default?

	Brian R. Bondy [:bbondy] Assignee
	Comment 15 • 11 years ago

That seems logical. will do.

	Jim Mathies [:jimm]
	Comment 16 • 11 years ago

Comment on attachment 750536 [details] [diff] [review]
Patch v1

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

::: browser/base/content/browser.js
@@ +1279,5 @@
>      setTimeout(function () { BrowserChromeTest.markAsReady(); }, 0);
>      TelemetryTimestamps.add("delayedStartupFinished");
> +
> +#ifdef XP_WIN
> +#ifdef MOZ_METRO

lets add a nice comment here explaining what this is.

@@ +1557,5 @@
> +      }
> +    }
> +
> +    pushDesktopControlledPrefToMetro(prefName);
> +    let self = this;

You aren't using 'self'.

::: browser/metro/base/content/browser-ui.js
@@ +245,5 @@
>  
>      return uri.spec;
>    },
>  
> +  _pullDesktopControlledPrefs: function() {

nit - comment header

	Brian R. Bondy [:bbondy] Assignee
	Comment 18 • 11 years ago

Fix for Bc test failure due to not removing the pref observer, carrying forward r+.

	Brian R. Bondy [:bbondy] Assignee
	Comment 19 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/6066b9d92032