Closed
Bug 861322
Opened 12 years ago
Closed 12 years ago
Make Metro read Desktop's app.update.enabled and app.update.auto settings
Categories
(Firefox for Metro Graveyard :: Install/Update, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 24
People
(Reporter: bbondy, Assigned: bbondy)
References
Details
Attachments
(2 files, 1 obsolete file)
6.38 KB,
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
7.53 KB,
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
Thoughts on Comment 0? It would avoid any new UX decisions.
Flags: needinfo?(robert.bugzilla)
Assignee | ||
Comment 2•12 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.
Comment 3•12 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?
Assignee | ||
Comment 4•12 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.
Assignee | ||
Comment 5•12 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.
Comment 6•12 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.
Assignee | ||
Comment 7•12 years ago
|
||
p=2 if rstrong agrees with the approach.
Comment 8•12 years ago
|
||
The pref has to be used to allow administrators to lock down the preference.
Flags: needinfo?(robert.bugzilla)
Comment 9•12 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.
Assignee | ||
Comment 10•12 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.
Comment 11•12 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.
Assignee | ||
Comment 12•12 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.
Updated•12 years ago
|
Component: General → Install/Update
OS: Windows 8 → Windows 8 Metro
Assignee | ||
Updated•12 years ago
|
Summary: Make Desktop's settings for updates per installation and stored in the registry instead of a pref → Make Metro read Desktop's app.update.mode setting
Assignee | ||
Comment 13•12 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.
Attachment #750536 -
Flags: review?(jmathies)
Comment 14•12 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?
Assignee | ||
Comment 15•12 years ago
|
||
That seems logical. will do.
Comment 16•12 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
Attachment #750536 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 17•12 years ago
|
||
Attachment #750536 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Summary: Make Metro read Desktop's app.update.mode setting → Make Metro read Desktop's app.update.enabled and app.update.auto settings
Assignee | ||
Updated•12 years ago
|
Attachment #750632 -
Flags: review+
Assignee | ||
Comment 18•12 years ago
|
||
Fix for Bc test failure due to not removing the pref observer, carrying forward r+.
Attachment #752507 -
Flags: review+
Assignee | ||
Comment 19•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Updated•10 years ago
|
OS: Windows 8 Metro → Windows 8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•