Closed
Bug 997332
Opened 10 years ago
Closed 10 years ago
Changing prefs in about:config often doesn't get saved
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(fennec+)
RESOLVED
FIXED
Firefox 31
Tracking | Status | |
---|---|---|
fennec | + | --- |
People
(Reporter: kats, Assigned: capella)
Details
(Whiteboard: [good-first-bug][lang=js])
Attachments
(1 file, 1 obsolete file)
963 bytes,
patch
|
wesj
:
review+
|
Details | Diff | Splinter Review |
Nexus 4, running recent m-c code: 1. Start fennec and load a page (https://people.mozilla.org/~kgupta/griddiv.html) 2. Observe that no tile borders (green lines overlayed on the content) are painted 3. Open a new tab and go to about:config 4. Find the layers.draw-tile-borders pref and toggle it to true 5. Observe that now there are green lines overlayed on everything 6. Close the about:config tab. Observe there are green lines on the other tab content 7. Use the Android app switcher and swipe away fennec to close it 8. Repeat step 1 Expected: Tile borders pref is still set to true and tile borders are still visible Actual: pref is reset back to false! :(
Reporter | ||
Updated•10 years ago
|
Version: unspecified → Trunk
Updated•10 years ago
|
tracking-fennec: --- → ?
Reporter | ||
Comment 1•10 years ago
|
||
It seems that this is somewhat intermittent. Sometimes the prefs get saved and sometimes they don't. I suspect we only flush to disk at particular times and it's possible to exit fennec without flushing them to disk. For example, if you replace step 7 with "adb install -r fennec.apk" then you can consistently reproduce this. I think it might be good to flush to disk when we unload the about:config tab, if that's possible.
Comment 2•10 years ago
|
||
can someone mentor this bug?
tracking-fennec: ? → +
Whiteboard: [good-first-bug][lang=js]
Assignee | ||
Comment 3•10 years ago
|
||
We could call Services.prefs.savePrefFile(null) in AboutConfig.uninit() (which is already triggered on page unload) ... but I don't think that helps when swiping close via app switcher
Reporter | ||
Comment 4•10 years ago
|
||
Note that I do close the about:config tab as part of the STR, so the unload handler on the page should be running. Of course it would it would be nice to not have to do that either.
Assignee | ||
Comment 5•10 years ago
|
||
So currently, pref changes are saved when Fennec moves to background by tapping the device menu before app-swiping close, but not when we finish about:config by tab-swipe close. The main confusing STR that I can repro is where a user will: 1) Make a pref change via: -Add new pref, -Reset-to-default by: button click, or, -Change-value by: toggle button click integer up/down arrow click integer/string value manual keying 2) Manually close the about:config tab 3) Install new version while Fennec still active. This patch will catch these situations .. we basically savePrefFile() whenever changes are made and about:config tab is closed. If there's another trappable way to force Fennec closed that avoids flushing prefs, I'll be happy to address that :-)
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #8409372 -
Flags: review?(wjohnston)
Assignee | ||
Comment 6•10 years ago
|
||
meh, (working too hard again) The patch is even simpler, as Preferences already avoids duplicate writes http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp?rev=e2c046c72d6c&mark=885-886#880 So we can just always do the call in uninit()
Attachment #8409372 -
Attachment is obsolete: true
Attachment #8409372 -
Flags: review?(wjohnston)
Attachment #8409385 -
Flags: review?(wjohnston)
Comment 7•10 years ago
|
||
Comment on attachment 8409385 [details] [diff] [review] bug997332.diff Review of attachment 8409385 [details] [diff] [review]: ----------------------------------------------------------------- I think we'd be well within our rights to do writes whenever prefs change in about:config. But this seems like a simpler fix. Lets see what the masses think :)
Attachment #8409385 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 8•10 years ago
|
||
Yah, I thought about that too but figured I'd be over-ruled on review to avoid (potential) extraneous updates ... "damned if yah do!" :-D https://hg.mozilla.org/integration/fx-team/rev/7bd04d2f2984
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7bd04d2f2984
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•