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.

Sign up for GitHub

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

Jump to bottom

Wrong cookies state in Brave shields after adding URL in brave://settings/cookies #24321

Closed
btlechowski opened this issue Jul 28, 2022 · 9 comments · Fixed by brave/brave-core#14449
Closed

Wrong cookies state in Brave shields after adding URL in brave://settings/cookies #24321

btlechowski opened this issue Jul 28, 2022 · 9 comments · Fixed by brave/brave-core#14449
Assignees
@boocmp
Labels
feature/cookie-allow-list feature/cookies feature/shields The overall Shields feature in Brave. OS/Desktop priority/P4 Planned work. We expect to get to it "soon". QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Test-Plan-Specified QA/Yes release-notes/exclude
Milestone
1.48.x - Release

Comments

@btlechowski
Copy link

Follow up to #8604

After adding URL in brave://settings/cookies, the state in Brave Shields does not reflect actual state

Steps to Reproduce

  1. Clean profile
  2. Open brave://settings/cookies
  3. add www.whatismybrowser.com in Sites than can always use cookies (do not check Including third-party cookies on this site)
  4. Open Return to www.whatismybrowser.com site and check cookie Shield settings

Actual result:

Brave Shields shows: Allow all cookies
But behaves differently (blocks 3rd party cookies)
image

Expected result:

Brave Shields shows: ??
Definitely not Allow all cookies as the definitions are different

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave 1.42.78 Chromium: 103.0.5060.134 (Official Build) beta (64-bit)
Revision 8ec6fce403b3feb0869b0732eda8bd95011d333c-refs/branch-heads/5060@{#1262}
OS Ubuntu 18.04 LTS

cc @rebron @boocmp @MadhaviSeelam @LaurenWags
@boocmp
Copy link

boocmp commented Jul 29, 2022

Yep, there is a bug in brave_shields_util.cc, should show 'block 3p' as it actually acts.

@boocmp boocmp self-assigned this Jul 29, 2022
@boocmp
Copy link

boocmp commented Jul 29, 2022

@rebron
Another case: we can "Allow all cookies" by default and then disable only first-party cookies for the site. The actual state will be "Allow only third party" (we haven't state for it in the bubble), in this case "Allow all cookies" will be displayed. What should we do?

@bridiver bridiver mentioned this issue Aug 1, 2022
Merged
25 tasks
@bridiver
Copy link
Contributor

bridiver commented Aug 1, 2022

The bug here is actually that shields reflects settings adding in brave://settings/cookies at all. Shields is a simplified UI by design and is not intended to cover the full range of all possible cookie settings. The shields UI should always show the value of the shields cookie settings which in this case would presumably be the default of block 3p. The setting in brave://settings/cookies will override that, but that does not change the value of the current shields cookie setting just like changing the shields cookie settings should not delete or change any settings manually added to brave://settings/cookies. For instance there is no equivalent shield setting to allow site.com including third-party cookies on this site because that translates into cookies for site.com being allowed whether they are first-party or third-party. The combination of shields settings and brave://settings/cookies will be applied correctly and non-default shields settings show up in brave://settings/cookies, but trying to create a drop-down option for every possible setting you can do in brave://settings/cookies is impractical and defeats the purpose.

@bridiver
Copy link
Contributor

bridiver commented Aug 1, 2022

@petemill re ^^ we could potentially show some kind of "warning" icon next to the shield settings if there are other settings that apply to the site in brave://settings/cookies

@iefremov
Copy link
Contributor

iefremov commented Aug 2, 2022 •
edited

we probably can ask @rebron about potential UX updates

@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Aug 2, 2022
@boocmp boocmp mentioned this issue Aug 4, 2022
Merged
25 tasks
@rebron rebron added this to the 1.48.x - Nightly milestone Dec 15, 2022
@bridiver
Copy link
Contributor

@btlechowski just to be clear this reverts an earlier incorrect change that showed the effective cookie setting instead of the brave shields cookie setting. The dropdown is now back to what it has always been before that and we should address what we want to do about the combination of chromium and brave cookie settings (either the warning icon or disable the ability to add chromium cookie settings) cc @pes10k

@MadhaviSeelam MadhaviSeelam added QA/In-Progress Indicates that QA is currently in progress for that particular issue and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jan 17, 2023
@MadhaviSeelam
Copy link

MadhaviSeelam commented Jan 18, 2023 •
edited

Verification PASSED using

Brave | 1.48.131 Chromium: 109.0.5414.87 (Official Build) beta (64-bit)
-- | --
Revision | 2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS | Windows 11 Version 21H2 (Build 22000.1455)

Case 1: Select cookies setting in brave://settings/cookies - Allow all cookies - PASSED

  1. Install 1.48.131
  2. launch Brave
  3. visit www.whatismybrowser.com in a new-tab
  4. confirmed default settings for cookies Block cross-site cookies displayed as expected
  5. open brave://settings/cookies in a new-tab
  6. Select Allow all cookies
  7. return to www.whatismybrowser.com site
  8. confirmed Allow all cookies displayed in the Shields panel
  9. open brave://settings/shields
  10. confirmed Disabled selection displayed for Block cookies option as expected
step 4 step 6 step 8 step 10
image image image image

Case 2: Select shields cookie setting in the Shields panel - Block all cookies - PASSED

  1. continue from Case 1:
    • brave://settings/cookies shows Allow all cookies
    • brave://settings/shields shows Disabled
    • Shields panel on whatismybrowser.com shows Allow all cookies
  2. return to whatismybrowser.com site
  3. select Block all cookies in the Shields panel
  4. refresh the page
  5. confirmed Block all cookies selection is retained
  6. visit brave://settings/cookies
  7. verified previous selection (Case 1) Allow all cookies is retained
  8. open brave://settings/shields
  9. verified previous selection (Case 1) Disabled is retained
  10. verified www.whatismybrowser.com is shown in Sites that can never use cookies
step 1 step 5 step 7 step 9 step 10
image image image image image

@MadhaviSeelam MadhaviSeelam removed the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jan 18, 2023
@stephendonner
Copy link

stephendonner commented Jan 18, 2023 •
edited

Verification PASSED using

Brave 1.48.129 Chromium: 109.0.5414.87 (Official Build) beta (x86_64)
Revision 2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS macOS Version 11.7.2 (Build 20G1020)

Case 1: Select cookies setting in brave://settings/cookies - Allow all cookies - PASSED

  1. installed 1.48.129
  2. launched Brave
  3. visited www.whatismybrowser.com in a new-tab
  4. confirmed default settings for cookies Block cross-site cookies displayed as expected
  5. opened brave://settings/cookies in a new tab
  6. selected Allow all cookies
  7. returned to www.whatismybrowser.com site
  8. confirmed Allow all cookies displayed in the Shields panel
  9. opened brave://settings/shields
  10. confirmed Disabled selection displayed for Block cookies option as expected
step 4 step 6 step 8 step 10
Screen Shot 2023-01-18 at 3 11 23 PM Screen Shot 2023-01-18 at 3 11 45 PM Screen Shot 2023-01-18 at 3 12 01 PM Screen Shot 2023-01-18 at 3 12 55 PM

Case 2: Select Shields cookie setting in the Shields panel - Block all cookies - PASSED

  1. continued from Case 1:
    • brave://settings/cookies shows Allow all cookies
    • brave://settings/shields shows Disabled
    • Shields panel on whatismybrowser.com shows Allow all cookies
  2. returned to whatismybrowser.com site
  3. selected Block all cookies in the Shields panel
  4. refreshed the page
  5. confirmed Block all cookies selection is retained
  6. visited brave://settings/cookies
  7. verified previous selection (Case 1) Allow all cookies is retained
  8. opened brave://settings/shields
  9. verified previous selection (Case 1) Disabled is retained
  10. opened brave://settings/cookies
  11. verified www.whatismybrowser.com is shown in Sites that can never use cookies
step 1 step 5 step 7 step 9 step 10
Screen Shot 2023-01-18 at 3 22 58 PM Screen Shot 2023-01-18 at 3 19 08 PM Screen Shot 2023-01-18 at 3 19 51 PM Screen Shot 2023-01-18 at 3 19 51 PM Screen Shot 2023-01-18 at 3 20 55 PM

@MadhaviSeelam MadhaviSeelam added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jan 25, 2023
@MadhaviSeelam
Copy link

MadhaviSeelam commented Jan 25, 2023 •
edited

Verification PASSED using

Brave | 1.48.140 Chromium: 109.0.5414.119 (Official Build) beta (64-bit)
-- | --
Revision | 772095164c7d5d4e73160f858efed3b5e87eca83-refs/branch-heads/5414@{#1458}
OS | Linux

Case 1: Select cookies setting in brave://settings/cookies - Allow all cookies - PASSED

  1. Install 1.48.140
  2. launch Brave
  3. visit www.whatismybrowser.com in a new-tab and click Shields panel
  4. confirmed default settings for cookies Block cross-site cookies displayed as expected
  5. open brave://settings/cookies in a new-tab
  6. Select Allow all cookies
  7. return to www.whatismybrowser.com site
  8. confirmed Allow all cookies displayed in the Shields panel
  9. open brave://settings/shields
  10. confirmed Disabled selection displayed for Block cookies option as expected
step 4 step 6 step 8 step 10
image image image image

Case 2: Select shields cookie setting in the Shields panel - Block all cookies - PASSED

  1. continue from Case 1:
    • brave://settings/cookies shows Allow all cookies
    • brave://settings/shields shows Disabled
    • Shields panel on whatismybrowser.com shows Allow all cookies
  2. return to whatismybrowser.com site
  3. select Block all cookies in the Shields panel
  4. refresh the page
  5. confirmed Block all cookies selection is retained
  6. visit brave://settings/cookies
  7. verified previous selection (Case 1) Allow all cookies is retained
  8. open brave://settings/shields
  9. verified previous selection (Case 1) Disabled is retained
  10. verified www.whatismybrowser.com is shown in Sites that can never use cookies
step 1 step 5 step 7 step 9 step 10
image image image image image

@MadhaviSeelam MadhaviSeelam added QA Pass-Linux and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@boocmp boocmp

Labels
feature/cookie-allow-list feature/cookies feature/shields The overall Shields feature in Brave. OS/Desktop priority/P4 Planned work. We expect to get to it "soon". QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Test-Plan-Specified QA/Yes release-notes/exclude
Projects
None yet
Milestone
1.48.x - Release
Development

Successfully merging a pull request may close this issue.

Show Brave cookies settings instead of effective cookie in Shields Bubble. brave/brave-core
8 participants
@bridiver @stephendonner @iefremov @rebron @LaurenWags @btlechowski @boocmp @MadhaviSeelam