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

about:adblock page with regional filters selection followup to #267 #1931

Closed
bbondy opened this issue Oct 31, 2018 · 6 comments · Fixed by brave/brave-core#2057
Closed

about:adblock page with regional filters selection followup to #267 #1931

bbondy opened this issue Oct 31, 2018 · 6 comments · Fixed by brave/brave-core#2057
Assignees
Labels
browser-laptop-parity design/needs-mock-up needs-mockup A feature which needs design mockup to be implemented. design A design change, especially one which needs input from the design team feature/global-settings Settings at browser level independent of shields settings feature/shields The overall Shields feature in Brave. priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/include

Comments

@bbondy
Copy link
Member

bbondy commented Oct 31, 2018

Description

Create an about:adblock page followup to #267. Add regional adblock list similar to muon build
image

Additional information

#267
Split from #403

@bbondy bbondy added the feature/shields The overall Shields feature in Brave. label Oct 31, 2018
@bbondy bbondy added the priority/P2 A bad problem. We might uplift this to the next planned release. label Oct 31, 2018
@tildelowengrimm tildelowengrimm added design A design change, especially one which needs input from the design team design/needs-mock-up needs-mockup A feature which needs design mockup to be implemented. priority/P3 The next thing for us to work on. It'll ride the trains. browser-laptop-parity and removed priority/P2 A bad problem. We might uplift this to the next planned release. labels Oct 31, 2018
@bbondy bbondy added this to the 1.x Backlog milestone Nov 5, 2018
@tildelowengrimm tildelowengrimm added the feature/global-settings Settings at browser level independent of shields settings label Nov 6, 2018
@tildelowengrimm
Copy link
Contributor

This should eventually live in global settings. Karen is going to produce some designs for the settings panel.

@bbondy
Copy link
Member Author

bbondy commented Nov 10, 2018

sounds good, there's going to be a lot of settings over time though, more than in browser-laptop too. uBlock has multiple tabs.

@tildelowengrimm
Copy link
Contributor

Yeah, I think that having a lot of settings is just the necessary consequence of having a lot of convoluted behaviors which folks might want to twiddle. I think we under-exposed advanced settings in Muon, which is part of the difference.

@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
@emerick emerick self-assigned this Mar 14, 2019
@emerick
Copy link
Contributor

emerick commented Mar 25, 2019

Our existing behavior (i.e., prior to any changes detailed in this ticket) is that on startup we retrieve the current application locale and start an AdBlock instance for that region if applicable. We also store this "current" AdBlock region in a preference, as there was only the concept of a single active region in this scenario.

How would we want to handle this going forward? The simplest thing would be to eliminate that preference completely and the regional filters are all initially disabled until a user enables them via the brave://adblock/ page. Alternatively we could try to be more clever and remain somewhat backward-compatible with the existing setting, "importing" it into our new regional settings on startup.

@bbondy any thoughts here?

@bbondy
Copy link
Member Author

bbondy commented Mar 27, 2019

Just following up for anyone following this issue, Emerick and I talked on Slack and discussed that we'd allow anything to be set, but that'd we'd auto turn on whatever matches the current language setting. The PR is updated with that.

@srirambv
Copy link
Contributor

srirambv commented May 8, 2019

Verification passed on

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

image

Verified passed with

Brave 0.64.72 Chromium: 74.0.3729.131 (Build officiel) beta (64 bits)
Révision 518a41c1fa7ce1c8bb5e22346e82e42b4d76a96f-refs/branch-heads/3729@{#954}
Système d'exploitation Mac OS X

Verification passed 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

Logged #4367
Tested that manually selecting FRA: EasyList Liste FR blocks more elements on https://www.recreatisse.com/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser-laptop-parity design/needs-mock-up needs-mockup A feature which needs design mockup to be implemented. design A design change, especially one which needs input from the design team feature/global-settings Settings at browser level independent of shields settings feature/shields The overall Shields feature in Brave. priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/include
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants