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

Cookies blocked doesn't work when selected via shields panel #17654

Closed
kjozwiak opened this issue Aug 24, 2021 · 3 comments · Fixed by brave/brave-core#9854
Closed

Cookies blocked doesn't work when selected via shields panel #17654

kjozwiak opened this issue Aug 24, 2021 · 3 comments · Fixed by brave/brave-core#9854
Assignees
@mkarolin
Labels
bug feature/shields/cookies Cookie controls implemented as part of Shields. OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes regression release-notes/exclude
Milestone
1.30.x - Release

Comments

@kjozwiak
Copy link
Member

Description

Selecting Cookies blocked via the shields panel doesn't block the cookies on the target website and generates the following error via the browser console:

setBlocked

Steps to Reproduce

  1. launch nightly with C93 (used 1.30.41 Chromium: 93.0.4577.51 in this case)
  2. visit https://jsfiddle.net/7ke9r14a/9/
  3. click on the shields panel and attempt to block cookies by selecting Cookies blocked

Actual result:

Nothing happens and you'll get the error mentioned above via the browser console when you inspect Brave via brave://inspect

Expected result:

Should block the entire website as per the following:

image

Reproduces how often:

100% reproducible using the above STR.

Brave version (brave://version info)

Brave | 1.30.41 Chromium: 93.0.4577.51 (Official Build) nightly (64-bit)
-- | --
Revision | 762d21050e2da59930c784c09b134d0b0b148188-refs/branch-heads/4577@{#915}
OS | Windows 10 OS Version 2009 (Build 19042.1165)

Version/Channel Information:

  • Can you reproduce this issue with the current release? No
  • Can you reproduce this issue with the beta channel? No
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

CCing @rebron @bsclifton @mkarolin @brave/legacy_qa
@kjozwiak kjozwiak added bug QA/Yes regression feature/shields/cookies Cookie controls implemented as part of Shields. OS/Desktop labels Aug 24, 2021
@kjozwiak kjozwiak mentioned this issue Aug 24, 2021
Closed
@kjozwiak kjozwiak added the priority/P2 A bad problem. We might uplift this to the next planned release. label Aug 24, 2021
@mkarolin mkarolin mentioned this issue Aug 25, 2021
Merged
24 tasks
mkarolin added a commit to brave/brave-core that referenced this issue Aug 25, 2021
@mkarolin
Reverts usage of JS APIs that return promises.
7d5228c 
These were introduced by #9545,
however, according to Chrome documentation, e.g.:
* https://developer.chrome.com/docs/extensions/reference/tabs/
* https://developer.chrome.com/docs/extensions/reference/browserAction/
the API versions that return Promise are only available to extensions
that use Manifest V3. We are using Manifest V2 and should instead be
using APIs that take a callback.

Replaced all usages of these APIs that expected a Promise returned with
the versions that take a callback.

Fixes brave/brave-browser#17654

Additionally, added some return statements inside .then() in
shieldsPanelReducer.ts to get rid of the Bluebird warnings that "a
promise was created in a handler but was not returned from it".
@mkarolin mkarolin self-assigned this Aug 25, 2021
@mkarolin mkarolin added this to the 1.30.x - Nightly milestone Aug 25, 2021
@mkarolin mkarolin mentioned this issue Aug 25, 2021
Merged
24 tasks
@kjozwiak
Copy link
Member Author

As per brave/brave-core#9832 (comment), I quickly double checked and ensured that I couldn't reproduce the above on 1.29.x as the fix wasn't uplifted into 1.29.x due to brave/brave-core#9545 not being in 1.29.x which was the cause of the original issue.

Using 1.29.74 Chromium: 93.0.4577.58, went through the STR/Cases outlined via #17654 (comment) and ensured that Cookies blocked was working as per the following:

image

@LaurenWags @StephenBass @btlechowski mind quickly checking just to confirm macOS & Linux are also working. I want to make sure this isn't an issue on 1.29.x as it's a pretty bad one.

@LaurenWags
Copy link
Member

Confirmed blocking cookies per the STR worked on

Brave | 1.29.74 Chromium: 93.0.4577.58 (Official Build) (x86_64)
-- | --
Revision | c4410ece044414ea42fa4ba328d08195e818a99c-refs/branch-heads/4577@{#1076}
OS | macOS Version 10.15.7 (Build 19H1323)

Screen Shot 2021-08-27 at 1 41 47 PM

@stephendonner
Copy link

stephendonner commented Aug 30, 2021 •
edited
Loading

Verified PASSED using

Brave 1.30.56 Chromium: 93.0.4577.58 (Official Build) beta (x86_64)
Revision c4410ece044414ea42fa4ba328d08195e818a99c-refs/branch-heads/4577@{#1076}
OS macOS Version 12.0 (Build 21A5304g)

Steps:

  1. new profile
  2. launched Brave
  3. loaded https://jsfiddle.net/7ke9r14a/9/
  4. selected Cookies blocked, Cross-site cookies blocked, and All cookies allowed, and confirmed all worked.
Cookies blocked Cross-site cookies blocked All cookies allowed
Screen Shot 2021-08-30 at 4 45 51 PM Screen Shot 2021-08-30 at 4 45 58 PM Screen Shot 2021-08-30 at 4 45 58 PM

Verification passed on

Brave | 1.30.59 Chromium: 93.0.4577.58 (Official Build) beta (64-bit)
-- | --
Revision | c4410ece044414ea42fa4ba328d08195e818a99c-refs/branch-heads/4577@{#1076}
OS | Windows 10 OS Version 2009 (Build 19043.1165)
  • Verified the STR from the description
Cookies blocked Cross-site cookies blocked All cookies allowed
image image image

Verified PASSED using

Brave 1.30.63 Chromium: 93.0.4577.63 (Official Build) beta (64-bit)
Revision ff5c0da2ec0adeaed5550e6c7e98417dac77d98a-refs/branch-heads/4577@{#1135}
OS Linux

Steps:

  1. new profile
  2. launched Brave
  3. loaded https://jsfiddle.net/7ke9r14a/9/
  4. selected Cookies blocked, Cross-site cookies blocked, and All cookies allowed, and confirmed all worked.
Cookies blocked Cross-site cookies blocked All cookies allowed
Screen Shot 2021-09-03 at 10 49 25 AM Screen Shot 2021-09-03 at 10 49 31 AM Screen Shot 2021-09-03 at 10 49 39 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mkarolin mkarolin

Labels
bug feature/shields/cookies Cookie controls implemented as part of Shields. OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes regression release-notes/exclude
Projects
None yet
Milestone
1.30.x - Release
Development

Successfully merging a pull request may close this issue.

Reverts usage of JS APIs that return promises. brave/brave-core
5 participants
@stephendonner @kjozwiak @LaurenWags @GeetaSarvadnya @mkarolin