Closed Bug 1118818 Opened 5 years ago Closed 5 years ago

Flush Gecko preferences when leaving GeckoPreferences

Categories

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

All
Android
defect

Tracking

()

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

People

(Reporter: rnewman, Assigned: rnewman, Mentored)

References

(Blocks 1 open bug)

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]
Duplicate of this bug: 1137153
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+
https://hg.mozilla.org/mozilla-central/rev/705bdc9370af
Status: ASSIGNED → RESOLVED
Closed: 5 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
Duplicate of this bug: 834444
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?
Marking 39 as affected.
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)
Duplicate of this bug: 1173388
You need to log in before you can comment on or make changes to this bug.