-
Notifications
You must be signed in to change notification settings - Fork 878
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
Tweak behavior of #brave-adblock-cookie-list-default flag #11621
Conversation
const bool cookie_list_touched = | ||
local_state->GetBoolean(prefs::kAdBlockCookieListSettingTouched); |
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.
Seems like a unit test to exercise this new preference could be useful.
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.
This is meant to be a pretty simple temporary change so we can use Griffin to evaluate whether or not it's worth pulling the cookie list into a better more permanent position, but point taken, I know how these "temporary" features go 😅
I couldn't see an easy way to test this in a unit test, but I've added a browsertest.
21dfc27
to
efaaf6f
Compare
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
efaaf6f
to
2b0a6fa
Compare
Windows error unrelated, merging. |
Resolves brave/brave-browser#20126
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Note: As of the time of writing, there is a filter issue preventing the space for the cookie banner from being collapsed when it appears at the top of the page. If that's still the case during testing, please consider the notice to be "not shown" so long as the text is not visible.
#brave-adblock-cookie-list-default
flag, then restart the browser.Easylist-Cookie List - Filter Obtrusive Cookie Notices
underAdditional filters
exists and is checked.#brave-adblock-cookie-list-default
toDisabled
, and restart the browser.Easylist-Cookie List - Filter Obtrusive Cookie Notices
is no longer checked.Easylist-Cookie List - Filter Obtrusive Cookie Notices
entry to be enabled, then repeat steps 2-5.Easylist-Cookie List - Filter Obtrusive Cookie Notices
is checked.Easylist-Cookie List - Filter Obtrusive Cookie Notices
entry to be disabled.#brave-adblock-cookie-list-default
flag, then restart the browser.Easylist-Cookie List - Filter Obtrusive Cookie Notices
is unchecked.