-
Notifications
You must be signed in to change notification settings - Fork 891
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
Support regional and custom cosmetic filters #4700
Conversation
e0cc15d
to
afc5d55
Compare
afc5d55
to
ca84eff
Compare
components/brave_shields/browser/ad_block_regional_service_manager.cc
Outdated
Show resolved
Hide resolved
components/brave_shields/browser/ad_block_regional_service_manager.cc
Outdated
Show resolved
Hide resolved
ca84eff
to
ef7f9b9
Compare
@emerick everything's updated according to your feedback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@antonok-edm besides automated tests, are there some manual test steps you can provide for QA to try this out? |
@antonok-edm is there an example from the crawl you did over the summer / autumn you could pull out and just copy-paste the URL here? I think that would suffice. |
@bsclifton @pes10k I've updated the Test Plan to include the example I used to make sure this works for custom filters. Unfortunately I haven't been able to find any good examples of up-to-date cosmetic filters in regional lists, but it uses the same machinery. |
Terrific! @bsclifton for my two cents, I think @antonok-edm's test plan is sufficient to make sure the relevant functionality is tickled |
Updated test plan looks perfect! 😄 |
ef7f9b9
to
e619115
Compare
Verification PASSED on
|
Resolves brave/brave-browser#4348
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
I have added some new unit tests to verify that the logic for merging cosmetic filter information from the different sources works as expected.
Manual verification of custom filter application:
myspace.com###articleLeft
Reviewer Checklist:
After-merge Checklist:
changes has landed on.