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.

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

allow first party subdomains when blocking 3rd-party cookies to match… #5318

Merged
merged 1 commit into from
Apr 22, 2020

Conversation

bridiver
Copy link
Collaborator

@bridiver bridiver commented Apr 22, 2020

… default 3rd-party cookie blocking policy

fix brave/brave-browser#9337

Resolves

Submitter Checklist:

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@bridiver bridiver self-assigned this Apr 22, 2020
@bridiver bridiver removed the request for review from iefremov April 22, 2020 18:55
@pes10k
Copy link
Contributor

pes10k commented Apr 22, 2020

lgtm

@bsclifton bsclifton added this to the 1.9.x - Beta milestone Apr 22, 2020
@bridiver bridiver requested a review from bbondy April 22, 2020 18:58
@bridiver
Copy link
Collaborator Author

unrelated test failures in CI

@bridiver bridiver merged commit 3711016 into master Apr 22, 2020
@bridiver bridiver deleted the issues/9337 branch April 22, 2020 23:42
bsclifton pushed a commit that referenced this pull request Apr 24, 2020
allow first party subdomains when blocking 3rd-party cookies to match…
bsclifton pushed a commit that referenced this pull request Apr 24, 2020
allow first party subdomains when blocking 3rd-party cookies to match…
bsclifton pushed a commit that referenced this pull request Apr 24, 2020
allow first party subdomains when blocking 3rd-party cookies to match…
@kjozwiak
Copy link
Member

kjozwiak commented Apr 26, 2020

Desktop Cases:

Reproduced the issue on macOS 10.15.4 x64 using the following build:

Brave | 1.10.3 Chromium: 81.0.4044.113 (Official Build) nightly (64-bit)
-- | --
Revision | e3225dafb0475864a1812a374d73a92e391635ac-refs/branch-heads/4044@{#936}
OS | macOS Version 10.15.4 (Build 19E287)

STR:

Screen Shot 2020-04-26 at 6 20 30 PM

Verification PASSED on macOS 10.15.4 x64 using the following build:

Brave | 1.10.11 Chromium: 81.0.4044.122 (Official Build) nightly (64-bit)
-- | --
Revision | 44f4233f08910d83b146130c1938256a2e05b136-refs/branch-heads/4044@{#963}
OS | macOS Version 10.15.4 (Build 19E287)
  • ensured that the above STR wasn't reproducible on a clean profile
    • ensured that switching between All Cookies Allowed & Cross-site cookies blocked didn't cause Twitter displaying Something went wrong as per the above.
  • ensured that upgrading from 1.10.3 Chromium: 81.0.4044.113 to a newer version of Nightly fixed the Something went wrong issue.
    • ensured that switching between All Cookies Allowed & Cross-site cookies blocked didn't cause any issues

@kjozwiak
Copy link
Member

I can still reproduce this on Samsung S10+ with Android 10 using the following build:

Brave - Nightly 1.10.11 CR: 81.0.4044.122

Example of the issue occurring: https://youtu.be/tTkfAaGITc0

@bridiver assuming this should have landed in Brave - Nightly 1.10.11 CR: 81.0.4044.122 as it's working on desktop via 1.10.11 Chromium: 81.0.4044.122.

@srirambv can you reproduce the issue on your device?

@bsclifton
Copy link
Member

@kjozwiak per commit log https://github.com/brave/brave-core/commits/37110161e042202ace67a2ea49991c7d97210d9c

This has been in since 1.10.5. So it's definitely there- not sure why Android isn't working though

@srirambv
Copy link
Contributor

Reproduced on 1.10.11 x64 nightly build. Noticed it sometimes starts working if I switch to desktop site.

@kjozwiak
Copy link
Member

kjozwiak commented Apr 27, 2020

@kjozwiak per commit log https://github.com/brave/brave-core/commits/37110161e042202ace67a2ea49991c7d97210d9c

This has been in since 1.10.5. So it's definitely there- not sure why Android isn't working though

yup, assumed once it landed it should be in both desktop and Android. Seems like this PR doesn't fix the problem on Android though. Looks like @srirambv can also reproduce as per the above. Possible related to desktop vs mobile modes as per above?

@GeetaSarvadnya
Copy link

Reproduce in Android - Brave 1.5.113 - Running on Samsung Galaxy J3
Reproduce in Android - Brave 1.10.11 - Running on Samsung Galaxy J3
Reproduce in Desktop - Brave Nightly 1.10.2 and Brave Nightly 1.10.3 -Running on Windows 10 x64 upgrade to Nightly 1.10.11 fixed the issue
Not Reproduced in Desktop - Brave Nightly 1.10.11 - Running on Windows 10 x64

bsclifton added a commit that referenced this pull request May 28, 2020
- test failure was caused by bad merge in #5410
- manually updated test with proper logic
- other tests had to be updated because 1.9 doesn't have #5318
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't load Twitter with 3rd party cookies blocked
7 participants