-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Iron out bug fixes before new release #407
Comments
In this issue I also created another change that could be making some problems, @sophieeng. The changes are the two functions |
I just pushed some changes that I will explain here. Previously, our Chrome GPC DOM signal worked by running two content scripts, one that added it and one that removed it. The script that added it had all sites matching, and to make a site remove the signal we would add that domain to the second content script and it would immediately strip that one. This method seems to have gotten less effective at consistently performing. Largely, this process was done in an inefficient manner where to update the second script we would iterate through our database looking for sites that were turned off and then setting the second script's matches to what we found. I just wrote up two more functions Another larger change is that I completely removed the second content script and instead am now utilizing the So far, the toggling on and off in the popup seems to be working for Chrome perfectly. Toggling on the domainlist seems to be working as well. However, the on/off popup button is not functioning as desired, so I will look into that further. It is definitely possible that in these changes one or two things fell through the cracks, so intensive testing would be helpful. In addition, our architecture readme will need to be updated to represent these changes, which I will open an issue for. |
Thanks for exploring this, @OliverWang13!
After your changes, would we still follow the same strategy of adding and removing or just adding (only the sites for which GPC is enabled)? To add also here, the reason was that sometimes our extension was not fast enough to set GPC on some sites. |
No, this will change the strategy so now we only add the GPC signal to the sites that are NOT specified in |
As @OliverWang13 clarified today, we are no longer using the two pronged strategy --- (1) adding GPC to all sites and (2) removing it from sites someone wants to remain opted in. This two-pronged strategy was initially helpful as we were not able to selectively attach GPC fast enough on some sites. We decided that it was better to having a user opted out on a site too many instead of having the user opted in on a site too many. The bottom line is that now simply adding GPC works. So, that is what we do. |
More detail on the last task to fix:
Here is what I notice on the Firefox extension using the toggle for the Do Not Sell signal:
|
Thanks, @sophieeng. That is strange, especially, as we have not switched to mv3 for Firefox yet. Can you look into other extensions, especially, DDG Privacy Essentials? Does it work for them? If so, what are they doing differently than we do? |
I believe I was able to resolve this Firefox header issue in the previous commit. There are no outstanding issues that I know of in the extension, and @sophieeng and I will run a few more final checks before finalizing the release. It is possible that an issue or two can pop up but currently our main extension functionality (headers, DOM) seems to be working properly. |
@sophieeng has tested the extension and has not found any new errors, so we will be closing this. |
Like it says in #382, we are now in the process of testing the extension and finding issues. Here are a few that I have noticed. @sophieeng, if you could run some more tests and add if you find anything, that would be great.
For now, this is all I see, but I expect some more to pop up.
The text was updated successfully, but these errors were encountered: