Opened on 05/11/2017 at 02:07:06 PM
Closed on 10/06/2017 at 08:12:24 AM
Last modified on 10/06/2017 at 07:59:33 PM
#5235 closed change (fixed)
Refactoring on SharedPrefs
Reported by: | diegocarloslima | Assignee: | diegocarloslima |
---|---|---|---|
Priority: | P4 | Milestone: | Adblock-Plus-for-Samsung-Browser-1.1.2 |
Module: | Adblock-Plus-for-Samsung-Browser | Keywords: | |
Cc: | jwangenheim, fhd | Blocked By: | |
Blocking: | Platform: | Samsung Browser | |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | yes |
Review URL(s): |
https://codereview.adblockplus.org/29449601/ |
Description
Background
We have some peculiarities while using SharedPrefs in ABP for Samsung Internet, such as using a string resource identifier as a key, gracefully handling different types storage (migration issues) and retrieving the shared prefs using the application context (related to #3931)
What to change
In order to have a more consistent usage of the SharedPrefs, we should create some utility methods that would wrap these peculiarities.
Attachments (0)
Change History (9)
comment:1 Changed on 05/26/2017 at 08:55:29 PM by diegocarloslima
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 07/19/2017 at 04:27:58 PM by abpbot
comment:3 Changed on 07/19/2017 at 04:28:54 PM by diegocarloslima
- Milestone set to Adblock-Plus-for-Samsung-Browser-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:4 Changed on 09/30/2017 at 04:06:51 AM by rscott
- Resolution fixed deleted
- Status changed from closed to reopened
"Configure automatic updates > Always" preference disappears on system language change
Version: adblockplussbrowser-debug.apk (1.1.2 candidate; md5sum: d165a476e198eb804a4ec4ff071f474e)
Samsung Internet version: 5.4.02.3
Platform: Pixel/Android 8.0
Reproduction:
- With device system language set to English, install the adblockplussbrowser 1.1.2 candidate APK.
- Tap "Configure automatic updates" > "Always".
- Back out of the app. Change device system language to French.
- Tap "Configurer les mises à jour automatique".
- Notice that no option in the radio buttons is set.
Expected result:
- Settings remain consistent regardless of changes to device language.
Additional information:
- This works correctly in 1.1.1, the current Play Store version of our app.
- I couldn't find any other setting that was similarly affected when changing the system language; even the other option in this radio button set (Configure automatic updates > On wi-fi only) seems to survive a language change intact.
comment:5 Changed on 10/05/2017 at 12:47:24 PM by jwangenheim
I found the problem and submitted a fix for codereview
comment:6 Changed on 10/05/2017 at 12:47:33 PM by jwangenheim
- Review URL(s) modified (diff)
- Status changed from reopened to reviewing
comment:7 Changed on 10/05/2017 at 01:20:11 PM by abpbot
A commit referencing this issue has landed:
Issue 5235 - Refactoring on SharedPrefs
comment:8 Changed on 10/06/2017 at 08:12:24 AM by jwangenheim
- Resolution set to fixed
- Status changed from reviewing to closed
comment:9 Changed on 10/06/2017 at 07:59:33 PM by rscott
- Verified working set
With the fix referenced above applied, this now works correctly! =)
A commit referencing this issue has landed:
Issue 5235 - Refactoring on SharedPrefs