-
Notifications
You must be signed in to change notification settings - Fork 905
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
Include shield content settings into cookie rules #3690
Conversation
Easier to review per-commit, since I've refactored and clang-formatted some tests |
6b4fc57
to
97543d1
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.
Changes worked great for me - I updated the issue to have an easy to test URL for knowing if cookie is blocked or not
I did notice one failing browser test though:
1 test failed:
BraveContentSettingsObserverBrowserTest.ShieldsDownOverridesBlockedCookies (../../brave/renderer/brave_content_settings_observer_browsertest.cc:639)
@bsclifton I believe it was fixed after the force push. I think we are safe to merge after the CI pass |
c2a8cb7
to
e65856c
Compare
Fix brave/brave-browser#6464
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.