We need to merge the Metro prefs: http://dxr.mozilla.org/mozilla-central/source/browser/metro/profile/metro.js To the Desktop profile: http://dxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js We should strive to not modify anything in how Desktop functions. For prefs that are in Desktop profile and Metro profile and are the same: No action For prefs that are in the Metro profile but not the Desktop profile: Search in dxr.mozilla.org and see if they are only used by Metro. If so, you can copy them over. If not, check what the default value is when the pref is not specified. If it is different from what we specify then we need to investigate that pref more. For prefs that are in the Desktop profile but not the Metro profile: There is probably no action needed, but it'd be good to do a search in dxr.mozilla.org to see if there are any adverse effects. For prefs that are in Desktop profile and Metro profile and are different: Is there a good reason for them to be different? If not use Desktop's pref value. If there is a good reason, split this pref to have a metro. prefix on it. In the code, dynamically check metroutils's isMetro property to determine which pref to use.

p=5

We should look at using different files for each browser. We can use the xre runtime call to flip between them. Prefs are pretty well encapsulated so it shouldn't be too hard to do.. http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp#1158

If any changes made by Firefox options or by the user in about:config would change the value in both environments, I'm in agreement with Jim. I think that is correct, but can you confirm Jim?

(In reply to Brian R. Bondy [:bbondy] from comment #3) > If any changes made by Firefox options or by the user in about:config would > change the value in both environments, I'm in agreement with Jim. I think > that is correct, but can you confirm Jim? That wouldn't happen. The two would be separate like they are now.

Are you sure? The prefs file itself lives within the installation directory, and any changes to that lives inside the profile as I understand it.

(In reply to Brian R. Bondy [:bbondy] from comment #5) > Are you sure? The prefs file itself lives within the installation directory, > and any changes to that lives inside the profile as I understand it. I'm suggesting we force this to be the case for both prefs files (defaults and user). If we split defaults and share user prefs things get a bit tricky when users change prefs that break desktop but work on metro and vice versa. Probably better to have separate pref files for both.

I think we should evaluate the amount of conflicts discovered in Comment 0 first. And then decide which route to go. There may be little to no conflicts that are important. If we end up going your route though, we can wontfix bug 924897

(In reply to Jim Mathies [:jimm] from comment #6) > (In reply to Brian R. Bondy [:bbondy] from comment #5) > > Are you sure? The prefs file itself lives within the installation directory, > > and any changes to that lives inside the profile as I understand it. > > I'm suggesting we force this to be the case for both prefs files (defaults > and user). This might not be needed. From testing I've found default prefs don't go into the profile. So maybe we only need to split up user prefs. This would save us from having to deal with a situation where users turn something on in one browser that breaks the other.

So looks like we're going to go ahead with using shared prefs code (see bug 931801) and keep pref files separate. Also changes to prefs should be stored separate. Pref code for this bug is around here: http://dxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp

So I tried to generalize this a bit to say touch instead of metro. Assuming maybe one day different OS will supports 2 apps, a touch based one and a non touch based one.

Comment on attachment 827175 [details] [diff] [review] touch-prefs.diff Review of attachment 827175 [details] [diff] [review]: ----------------------------------------------------------------- This basically just mirrors the same functionality of NS_APP_PREFS_50_FILE

(In reply to Brian R. Bondy [:bbondy] from comment #10) > Created attachment 827175 [details] [diff] [review] > touch-prefs.diff > > So I tried to generalize this a bit to say touch instead of metro. Assuming > maybe one day different OS will supports 2 apps, a touch based one and a non > touch based one. Hmm, not a fan of this naming. I think we can have something specific to the 'immersive' or 'metro' browser in dir service. We could also name it something like 'alt'. In dir service I'm a fan of getting specific('MetroPrefF'). This service is already confusing enough. Also for users who like to poke around profiles, 'touch-app-prefs.js' doesn't do a good job of explaining what this prefs file is. bsmedberg owns this area of the code base, I'd be interested in his thoughts. (Also, please comment the define entries regardless of what we go with.)

Hey Jim, all those changes sound good. Would you be opposed to this plan: 1. I'd make your suggested changes. 2. I'd request review from you again. 3. You'd review using your best judgement. 4. I'd go ahead and land if/when it reaches r+. 5. I'd re-set a new needinfo flag for bsmedberg, and if there is any negative feedack there I'd file a followup bug to address it. This would keep things moving and ensure this gets in for this week's iteration testing. Any fallout even if we needed to do a full backout would be minimal and we could live with it on Nightly.

Review comments addressed.

Comment on attachment 827368 [details] [diff] [review] rev2 - Naming w/ Metro and commented define wfm, thanks!

Reducing from p=5 to p=3, was easier than expected. Moving extra 2 points to the contingency bug.

Comment on attachment 827368 [details] [diff] [review] rev2 - Naming w/ Metro and commented define Do we need to have different keys? That is, does the desktop side need to know where the metro prefs file is, and does the metro side need to know where the desktop side is? I suspect that rather than adding a new key, it would be better to just change the location of the existing key based on what runtime mode we're in. That's the whole point of the dirservice anyway.

Thanks for the quick feedback. We don't currently need to access the Metro prefs file from Desktop, and vice versa, and I'm not sure if we will have that need. But what you're saying sounds best to me and I'll file a followup to do that, not of critical priority or anything, but I will get to it when I have time.

For this, I've tried the following: - in desktop mode, from about:config, I've changed nglayout.debug.paint_flashing to false, and then opened the Metro mode - results: in desktop mode, I can see a bunch of colors flashing, but in Metro mode that doesn't happen Besides the above example, could you please provide some guidance in order for QA to test this as thorough as needed? Thanks!

Forgot to mention in comment 21, I've tested this with the latest oak build, on Win 8 64-bit.

You can do the same but the opposite, set something in Metro and then check Desktop. Also you can verify in about:config from both environments to see if the value gets changed or not. For almost all prefs (except a list of a few prefs I sync on purpose), they shouldn't reflect in the opposite environment.

Verified as fixed, on Win 8 64-bit, with both latest Oak and Nightly builds, following the suggestions from comment 21 and comment 23. Thanks Brian! :)

For iteration #19, verified as fixed, on Win 8 64-bit, with both latest Oak and Nightly builds, following the suggestions from comment 21 and comment 23. In comment 21 I meant to say that I've changed the nglayout.debug.paint_flashing pref to "true" (not "false").