Settings will not persist in Metro Firefox when you follow these steps: 1. Open Metro Firefox 2. Change some settings 3. Go to Desktop 4. Expand the application bar on the left and locate Metro Firefox 5. Right click on Metro Firefox and click the close menu 6. Re-open Metro Firefox Actual Results: - No settings are persisted. Expected Results: - Settings should be persisted.

For consideration in it5

When closing Firefox in this way we basically don't get a clean shutdown. Maybe the fix here is to save after dismissing the settings pane instead of on app shutdown.

Unassigned myself for now, but if no one else wants it I'll take it.

Being placed into Story Backlog based on Asa's review.

Point Estimation =2.

This patch listens saves preferences when the metro process is suspended. This patch modifies metro's browser.js to listen for process suspension, but really I'm thinking that the preferences service should do this at [1]. I'm not sure how that will affect other platforms, so maybe let's land this patch and file a follow-up to modify the preferences service? [1] https://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp?rev=36a1a6b81f90#405

Comment on attachment 745323 [details] [diff] [review] patch v1 Review of attachment 745323 [details] [diff] [review]: ----------------------------------------------------------------- We'll be saving it sometimes when we don't need to but I don't think that matters too much since it's on suspend. Thanks! > I'm not sure how that will affect other platforms, so maybe let's land this patch and file a follow-up to modify the preferences service? Improper shutdowns like this from the application bar is unique to windows so we don't have to worry about other platforms w/ -metrodesktop. suspend_process_notification will only be fired from windows so this patch has pretty much no effect on those other platforms, which is fine. ::: browser/metro/base/content/browser.js @@ +610,5 @@ > */ > _handleCertException: function _handleCertException(aMessage) { > let json = aMessage.json; > if (json.action == "leave") { > + // Get the start page from the *default* prefbranch, not the user's nit: I think I prefer pref branch

(In reply to Brian R. Bondy [:bbondy] from comment #7) > Comment on attachment 745323 [details] [diff] [review] > patch v1 > > Review of attachment 745323 [details] [diff] [review]: > ----------------------------------------------------------------- > > We'll be saving it sometimes when we don't need to but I don't think that > matters too much since it's on suspend. Thanks! > > > I'm not sure how that will affect other platforms, so maybe let's land this patch and file a follow-up to modify the preferences service? > > Improper shutdowns like this from the application bar is unique to windows > so we don't have to worry about other platforms w/ -metrodesktop. > suspend_process_notification will only be fired from windows so this patch > has pretty much no effect on those other platforms, which is fine. Ah, then maybe I can go ahead and change the prefs service instead of modifying browser.js? The only reason I was thinking that this change should go in the prefs service instead of browser.js is that there are other shared services that handle the message directly [1][2][3] instead of us doing the processing in metro-specific code. > ::: browser/metro/base/content/browser.js > @@ +610,5 @@ > > */ > > _handleCertException: function _handleCertException(aMessage) { > > let json = aMessage.json; > > if (json.action == "leave") { > > + // Get the start page from the *default* prefbranch, not the user's > > nit: I think I prefer pref branch ha, I'm not even sure how that change got in there. I guess a cat stepped on my keyboard while I had that file open in a text editor :) [1] https://mxr.mozilla.org/mozilla-central/source/xpcom/threads/TimerThread.cpp?rev=fb7aae8421bc#417 [2] https://mxr.mozilla.org/mozilla-central/source/netwerk/cache/nsCacheService.cpp?rev=e73333270ce5#408 [3] https://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/nsDownloadManager.cpp?rev=c8b3e4b6a8b5#2400

Ya I added those calls, I think that's a good suggestion but it may take a while to get a review if you do it that way. I'm ok with landing it this way to save time, but I'll leave it up to you.

This patch save preferences when the process is being suspended. Benjamin: You're listed as a Preferences peer. Dan Witte is listed as the module owner, but his bugzilla name indicates that he doesn't read bugmail. Please feel free to forward this review to whomever makes sense/has time.

For testing and verification.

Mozilla/5.0 (Windows NT 6.2; rv:24.0) Gecko/20130526 Firefox/24.0 Could still reproduce with my older profile (used since before the fix), but it works on a clean one. So I'll set this to verified. - settings (Startup, Character Encoding, Password and DNT) are remembered when exiting Metro Firefox normally through drag and drop. - settings (Startup, Character Encoding, Password and DNT) are remembered when exiting Metro Firefox through the application bar