Don't check for supercookies until document_idle #1522
Conversation
There was a problem hiding this comment.
Unlike fingerprinting detection, where our injection needs to happen before anything on the page does, "supercookie" detection merely iterates over localStorage entries, which we should be able to do at any point.
There are probably differences in what we find and/or how soon we find it when we iterate later rather than sooner. Maybe we'll pick up more localStorage than before with this change, because we'd notice localStorage written by a script within the same page load.
Injecting later might also have a beneficial effect on page load, although maybe not noticeable.
@cowlicks Could you run Privacy Badger through a set of URLs, the larger the better (with badgers.cowlicks.website?), and compare supercookie-attributed tracking domains we find before this patch to the supercookie-attributed tracking domains after this patch? You'd probably have to tweak Privacy Badger to store or log this extra information since we don't yet differentiate between the different kinds of tracking a domain may perform.
|
Regarding the test that keeps failing on Firefox ESR, are you able to reproduce the failures when running the test locally with ESR? |
|
This also fixes how With this patch: Without it: |
|
Demonstrating it fixes #1483 |
|
I'm happy to do some crawls with this patch later, to measure how much more effective this fix is. But since it has already been demonstrated to be effective, useful, and not break anything, I'm merging it. Yay! No more broken tests. |
|
How did we demonstrate this doesn't break anything? Having tests pass is not enough, we still need to do manual testing. |
|
@ghostwords do you have a specific concern about what this might break? I'm happy to add an additional test. I don't see how it could break anything other than the localstorage tracking detection, but I've manually tested that still works by changing the tests to run the previously broken tests repeatedly. |
|
My point is that this simple change actually has a big effect on Privacy Badger's functioning, specifically via the localStorage tracking detection component, and that having tests pass doesn't tell us what kind of effects, positive or negative, this change will have on actual user experience. On the other hand, performing a couple of crawls, automated or not, and examining the results (here focusing on localStorage tracking detection) before and after the change will give us a pretty good idea of how we did. This is something you should be able to do manually (with a few code tweaks to log localStorage findings for review) in a matter of hours. |
This should close #1483
My theory is that there is a race condition. The supercookie detection content script is injected at "
document_start" which means it might check for the localstorage data before the other script can write it.