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

Fix cookies are not imported from Chrome #3804

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Oct 29, 2019

Since the V11 cookies db schema, firstpartyonly column is renamed to samesite.
Also, BraveImporter and ChromeImporter should have different cookies query
string because muon uses old schema.

Upstream changes: https://chromium-review.googlesource.com/c/chromium/src/+/1570416

I think we should update test/data/import/chrome/default/Cookies periodically for testing with latest chrome cookies db.

Fix brave/brave-browser#5975
Fix brave/brave-browser#5313

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.

@simonhong simonhong added this to the 0.73.x - Nightly milestone Oct 29, 2019
@simonhong simonhong requested a review from bsclifton October 29, 2019 00:45
@simonhong simonhong self-assigned this Oct 29, 2019
@simonhong
Copy link
Member Author

simonhong commented Oct 29, 2019

Oh, I found BraveImporterTest.ImportCookies failed. checking now.
Maybe its test cookies db should be updated also.
-> fixed.

@simonhong simonhong force-pushed the fix_cookie_import_error_from_chrome branch from 33d2d36 to 32e606f Compare October 29, 2019 01:47
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with GitHub, importing from Canary. Works great! 😄 Super nice to have this fixed- thanks! 😄👍

Since the V11 cookies db schema, firstpartyonly column is renamed to samesite.
Also, BraveImporter and ChromeImporter should have different cookies query
string because muon uses old schema.
@simonhong simonhong force-pushed the fix_cookie_import_error_from_chrome branch from 32e606f to bc513db Compare October 29, 2019 08:52
@simonhong
Copy link
Member Author

Merged because CI has some internal error node disconnecting and I saw all binaries are built for all platforms from multiple CI trying.

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.

Cookies not imported from Chrome .google.com cookie mismatch after import
2 participants