-
Notifications
You must be signed in to change notification settings - Fork 910
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
Set new consent cookie and supress tracking cookies on the watch page #4013
Conversation
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.
LGTM
Could you explain the implications of this change and/or share a resource or another example of this? It's not that I don't trust you, but I'm not currently able to review this with knowledgeable confidence (due to my own ignorance on the subject). |
yt-dlp uses the SOCS cookie too now (fixed some extraction errors for them), as for nuking tracking cookies before Electron can save them, so that it doesn't send them along with future requests, that's something that should be done regardless of whether you want the new consent cookie or not. |
PR for |
I got 1 extra: |
This pull request should block it, try restarting FT, after switching to this branch. |
When I review a PR I always kill Electron (normal quit or force kill), check out branch via Maybe additional steps required to clear cookies? (no idea) |
To be sure it isn't my local env screwed up I tried Maybe I need to manually remove that key? |
development has the cookies, this pull request aims to get rid of them, weird that some are gone for you but others are still there |
I think you need to add code to remove existing cookies |
@PikachuEXE The problem is that you are quitting FreeTube instead of closing all windows. When you close the last window FreeTube clears up browsing data including cookies, we can't run it when quitting because it's an asynchronous operation but the quit event handler only waits for synchronous code. https://github.com/FreeTubeApp/FreeTube/blob/development/src/main/index.js#L1035-L1061 This sounds like an issue that will be exclusive to macOS as quitting without closing the windows isn't possible on Windows or Linux (unless you force kill it in the task manager but then you'll be fully aware that the app hasn't had time to clean up). |
https://www.electronjs.org/docs/latest/api/app#event-window-all-closed
See if you want this issue to be fixed in another PR or not |
I have updated code locally and tested Code let resourcesCleanUpDone = false
app.on('window-all-closed', () => {
// Clean up resources (datastores' compaction + Electron cache and storage data clearing)
cleanUpResources().finally(() => {
if (process.platform !== 'darwin') {
app.quit()
}
})
})
if (process.platform === 'darwin') {
// `window-all-closed` will no fire on Cmd+Q
// https://www.electronjs.org/docs/latest/api/app#event-window-all-closed
// It's also fired when `app.quit` called
// Not using `before-quit` since that one is fired before windows are closed
app.on('will-quit', (e) => {
// Let app quit of resource cleanup done
if (resourcesCleanUpDone) { return }
e.preventDefault()
cleanUpResources().finally(() => {
// Auto quit again AFTER resource cleanup done
// Which will call this listener again
app.quit()
})
})
}
function cleanUpResources() {
if (resourcesCleanUpDone) {
return Promise.resolve()
}
return Promise.allSettled([
baseHandlers.compactAllDatastores(),
session.defaultSession.clearCache(),
session.defaultSession.clearStorageData({
storages: [
'appcache',
'cookies',
'filesystem',
'indexdb',
'shadercache',
'websql',
'serviceworkers',
'cachestorage'
]
})
])
.then(() => {
resourcesCleanUpDone = true
})
} My test steps:
Not tested in other OSes |
@absidue are you planning on adding the macos code to this pr? |
I can put it into a new PR if you prefer |
I can add it to this one, I just can't test it. |
Please add and I will test it again |
Co-Authored-by: PikachuEXE <pikachuexe@gmail.com>
Updated this pull request. |
Re-tested on MacOS with steps in |
@ChunkyProgrammer @jasonhenriquez please review when u have time |
* development: IV: Show channel handle in search results (FreeTubeApp#3791) Bump electron from 22.3.25 to 22.3.26 (FreeTubeApp#4120) Bump lefthook from 1.5.0 to 1.5.2 (FreeTubeApp#4122) Bump the eslint group with 4 updates (FreeTubeApp#4119) Bump sass from 1.68.0 to 1.69.0 (FreeTubeApp#4121) Bump marked from 9.0.3 to 9.1.0 (FreeTubeApp#4123) Bump version number to v0.19.1 Fix styling for Channel page on desktop view (FreeTubeApp#4112) Set new consent cookie and supress tracking cookies on the watch page (FreeTubeApp#4013) Translated using Weblate (Italian) Update translation files Added translation using Weblate (Belarusian)
Set new consent cookie and supress tracking cookies on the watch page
Pull Request Type
Description
This pull request set the new consent cookie, based on what yt-dlp uses, although the old cookie still seems to be used too.
It also gets rid of tracking cookies that YouTube is returning, maybe we should consider changing the values of the consent cookies at some point, so that YouTube doesn't return those cookies.
Screenshots
before:

after:

Testing
https://www.youtube.com/sw.js_data
andhttps://www.youtube.com/iframe_api
CONSENT
andSOCS
cookies are listed