Closed Bug 1118818 Opened 10 years ago Closed 10 years ago

Flush Gecko preferences when leaving GeckoPreferences

Categories

(Firefox for Android Graveyard :: Settings and Preferences, defect, P5)

All
Android
defect

Tracking

(firefox39+ verified, firefox40 fixed, fennec39+)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox39 + verified
firefox40 --- fixed
fennec 39+ ---

People

(Reporter: rnewman, Assigned: rnewman, Mentored)

References

Details

(Whiteboard: [good next bug][lang=java])

Attachments

(1 file)

This will mitigate Bug 1025560. See that bug for context.
I am the user who posted the initial complaint. Please note that in my original sequence of steps I did not leave GeckoPreferences before killing the app. Working around by installing the "Quit" add-on and using it to exit Firefox after setting preferences works fine for me.
This is pretty much a one-liner, by the way. We can do better than "on leaving" by flushing whenever you change a pref while Settings is open, if we really really care about the durability.
Whiteboard: [good next bug][lang=java]
tracking-fennec: --- → 39+
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
This is the simplest and most durable option: when Settings (GeckoPreferences) sends a Preferences:Set message to Gecko, typically via PrefsHelper and a SharedPreferences change observer, we include a "flush": true field. When browser.js handles Preferences:Set, it checks for this and flushes. The scope of this change, then, is preferences changes within Settings; nothing else (e.g., turning on search suggestions) is affected. I decided to do this rather than sending a flush message when leaving Settings for three reasons: * One less message. * No flushes if nothing changed. * No data loss if we crash before the user completes the departure from Settings (e.g., they change something then hit the Android 'home' button).
Attachment #8588480 - Flags: review?(bnicholson)
Comment on attachment 8588480 [details] [diff] [review] Flush Gecko preferences when they change in Settings. v1 Review of attachment 8588480 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/chrome/content/browser.js @@ +1469,5 @@ > Services.prefs.setComplexValue(json.name, Ci.nsISupportsString, pref); > break; > } > } > + trailing whitespace
Attachment #8588480 - Flags: review?(bnicholson) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Trying the steps from Bug 1137153 with latest Nightly (2015-04-16) on Alcatel one Touch (Android 4.1.2) I still reproduce the issue: 1. Open Firefox (see that Google is default search engine) 2. Change default search engine to "Bing" 3. Perform a search with "Bing" 4. Crash Firefox (for example with: http://people.mozilla.org/~tmielczarek/crashme/ ) 5. Restart Firefox and go to Settings -> Customize -> Search => "Google" is listed as default search engine instead of "Bing" Trying the steps from Bug 1025560 #c23 with latest Nightly (2015-04-16) on Alcatel one Touch (Android 4.1.2) I cannot reproduce the issue: 1. Go to Settings -> Customize -> Search -> check "Show search suggestions" option 2. Install crash-me add-on: http://people.mozilla.org/~tmielczarek/crashme/ 3. Go to Menu -> Crash me! 4. Restart Firefox and go to Settings -> Customize -> Search => "Show search suggestions" option is checked
Turns out that SearchEngineCategory doesn't use the same prefs message, so un-duping :) The fix in that bug will be to apply a similar change to SearchEngines:SetDefault and SearchEngines:RestoreDefaults.
Blocks: 1173388
Comment on attachment 8588480 [details] [diff] [review] Flush Gecko preferences when they change in Settings. v1 Approval Request Comment [Feature/regressing bug #]: Bug 1173388 was filed, and seems to be the beta/39 version of this bug [User impact if declined]: Users using some devices running Android 4.2 will not be able to save their preferences if they change them and restart them [Describe test coverage new/current, TreeHerder]: Landed in 40 [Risks and why]: low, this is uplifting a patch that has been in tree for a full release, and just adding an extra parameter to a JSON object [String/UUID change made/needed]: none
Attachment #8588480 - Flags: approval-mozilla-beta?
Comment on attachment 8588480 [details] [diff] [review] Flush Gecko preferences when they change in Settings. v1 Fixes a regression in 39 (from bug 1173388), has been on aurora with no problems.
Attachment #8588480 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I'm not able to reproduce this issue. The changes in settings are preserved, except bug #1137153 Verifying as fixed on Firefox 39.0b7 on Galaxy Note 3 (4.4.2)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: