Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regional adblock filters are not pre-selected based on locale #4367

Closed
btlechowski opened this issue May 9, 2019 · 13 comments · Fixed by brave/brave-core#17261
Closed

Regional adblock filters are not pre-selected based on locale #4367

btlechowski opened this issue May 9, 2019 · 13 comments · Fixed by brave/brave-core#17261
Assignees
Labels

Comments

@btlechowski
Copy link

btlechowski commented May 9, 2019

Follow up to #1931

Steps to Reproduce

1, Start with a clean profile in pl locale
2. Set locale to "fr"
3. Restart Brave
4. Visit brave://adblock
5. Verify that French regional list is selected

Actual result:

FRA: EasyList Liste FR is not pre-selected

image

Expected result:

FRA: EasyList Liste FR is pre-selected
image

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Reproduced on

Brave 0.64.72 Chromium: 74.0.3729.131 (Official Build) beta(64-bit)
Revision 518a41c1fa7ce1c8bb5e22346e82e42b4d76a96f-refs/branch-heads/3729@{#954}
OS Windows 10 OS Build 17134.523

Not reproducible on

Brave 0.63.55 Chromium: 74.0.3729.131 (Official Build) (64-bit)
Revision 518a41c1fa7ce1c8bb5e22346e82e42b4d76a96f-refs/branch-heads/3729@{#954}
OS Windows 10 OS Build 17134.523

image

So this is a regression.

Note: also reproduced with Polish language. I also changed system locale, but it didn't fix the issue

cc @brave/legacy_qa @emerick

@emerick
Copy link
Contributor

emerick commented May 9, 2019

@btlechowski Just double-checking: When you set the locale to "fr", did you also check the option for "Display Brave in this language"? You'll know when you've properly done it, because Brave will then show a button to relaunch the browser. Sorry if you have done this, just wanted to confirm because it's easy to miss.

@btlechowski
Copy link
Author

btlechowski commented May 9, 2019

@emerick Yes, I did set the language and restarted the browser as you described. Brave was launched in french locale.

@emerick
Copy link
Contributor

emerick commented May 23, 2019

Had a little bit of time to dig into this. When we pre-select the regional adblock filter based on the current locale, we only perform that check once at startup (otherwise, we'd end up always selecting that region even if the user decided to disable it in brave://adblock...) This means that in order to test this feature, the first run of Brave with the new profile must already be using the desired region.

I suggest testing it like this, at least on Windows:

  1. Clear the profile
  2. Start brave with the --lang=fr flag (or whatever other language)
  3. Verify that brave://adblock shows the French list is selected
  4. Exit brave
  5. Start brave with a different language (e.g., --lang=de)
  6. Verify that brave://adblock still shows the French list is selected
  7. Unselect the french list
  8. Start brave with the --lang=fr flag
  9. Verify that brave://adblock still shows the French list is unselected

Something like that.

@tildelowengrimm tildelowengrimm added the priority/P4 Planned work. We expect to get to it "soon". label Jun 14, 2019
@btlechowski
Copy link
Author

@jumde @antonok-edm Is this issue still a concern?
The list changed significalty:
image

Brave 1.13.79 Chromium: 85.0.4183.69 (Official Build) dev (64-bit)
Revision 4554ea1a1171bd8d06951a4b7d9336afe6c59967-refs/branch-heads/4183@{#1426}
OS Ubuntu 18.04 LTS

cc @rebron

@antonok-edm
Copy link
Collaborator

@btlechowski those lists are dynamically populated based on https://github.com/brave/adblock-resources/blob/master/filter_lists/regional.json.

The behavior should still be the same though, if the locale is fr then AdGuard Français should be automatically selected at startup because it appears here - if not, that's still a bug.

@antonok-edm
Copy link
Collaborator

It turns out we have a langs field in https://github.com/brave/adblock-resources, and as a result we were trying to use language codes rather than locale codes to preselect filter lists in brave://adblock.

Two of these were addressed in brave/adblock-resources#56, although I haven't checked if that is an exhaustive fix.

@btlechowski
Copy link
Author

@antonok-edm When the fix will be available in nightly?

@antonok-edm
Copy link
Collaborator

@btlechowski the fix I mentioned above is just for the Japanese and Indian lists, but that's already available everywhere (shipped as a component update).

If there are still other locales that should be pre-selecting lists and aren't, let me know and I can push out a similar update for those as well.

@btlechowski
Copy link
Author

Still reproducible

Brave 1.37.46 Chromium: 98.0.4758.87 (Official Build) nightly (64-bit)
Revision e4cd00f135fb4d8edc64c8aa6ecbe7cc79ebb3b2-refs/branch-heads/4758@{#1002}
OS Windows 7 Service Pack 1 (Build 7601.24544)

After changing language to CZE, Easylist-Cookie List - Filter Obtrusive Cookie Notices was added to the list
I would expect CZE, SVK: EasyList Czech and Slovak to be add. This could be related to #20825

image

@stephendonner
Copy link

Verified PASSED using

Brave 1.50.91 Chromium: 111.0.5563.64 (Official Build) beta (x86_64)
Revision c710e93d5b63b7095afe8c2c17df34408078439d-refs/branch-heads/5563@{#995}
OS macOS Version 11.7.4 (Build 20G1120)

Steps:

  1. installed 1.50.91
  2. set operating system's region to the respective locale, from below
  3. restarted the OS
  4. launched Brave
  5. opened brave://adblock
  6. confirmed each locale's default, pre-selected list(s)
fr pl cze ja
Capture d’écran 2023-03-14 à 17 07 55 Zrzut ekranu 2023-03-14 o 17 16 14 Snímek obrazovky 2023-03-14 v 17 23 28 スクリーンショット 2023-03-14 17 29 23

@btlechowski
Copy link
Author

Verification passed on

Brave 1.50.93 Chromium: 111.0.5563.64 (Official Build) beta (64-bit)
Revision c710e93d5b63b7095afe8c2c17df34408078439d-refs/branch-heads/5563@{#995}
OS Ubuntu 18.04 LTS

Set locale to 'pl' and verified PL lists are set on clean profile
image

Changed locale to 'fr' and verified only 'fr' lists are set - FAILED

image

Logged: #29249

@MadhaviSeelam
Copy link

MadhaviSeelam commented Mar 29, 2023

Verification PASSED using

Brave | 1.50.107 Chromium: 112.0.5615.39 (Official Build) beta (64-bit)
-- | --
Revision | a0e7b9718a92bcd1cf33b7c95316caff3fc20714-refs/branch-heads/5615@{#753}
OS | Windows 11 Version 22H2 (Build 22621.1413)

Pre-requisites

  • Configure Brave to use Polish locale using the language code pl , es, ru by following below steps:
    • Browse Brave's regional list catalog here and find a language short-code used by more than one list (e.g. pl, is, es).
    • Configure Brave to use a locale with the selected language short-code according to instructions here

Steps:

  1. installed 1.50.107
  2. complete pre-requisite steps from above for each locale
  3. set operating system's region to the respective locale, from below
  4. restarted the OS
  5. launched Brave
  6. opened brave://adblock
  7. confirmed each locale's default, pre-selected list(s)
es 'pl` ru ko
image image image image

@srirambv
Copy link
Contributor

srirambv commented Apr 3, 2023

Verification passed on Oppo Reno 5 with Android 13 running 1.50.110 x64 build

4367-ARM.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants