Opened 2 years ago

Closed 21 months ago

Last modified 21 months ago

#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/
https://codereview.adblockplus.org/29565696/

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.

Change History (9)

comment:1 Changed 2 years ago by diegocarloslima

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5235 - Refactoring on SharedPrefs

comment:3 Changed 2 years ago by diegocarloslima

  • Milestone set to Adblock-Plus-for-Samsung-Browser-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:4 Changed 21 months ago 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 21 months ago by jwangenheim

I found the problem and submitted a fix for codereview

comment:6 Changed 21 months ago by jwangenheim

  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

comment:7 Changed 21 months ago by abpbot

A commit referencing this issue has landed:
Issue 5235 - Refactoring on SharedPrefs

comment:8 Changed 21 months ago by jwangenheim

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:9 Changed 21 months ago by rscott

  • Verified working set

With the fix referenced above applied, this now works correctly! =)

Note: See TracTickets for help on using tickets.