search

privacy-tech-lab / privacy-pioneer

Privacy browser extension for analyzing web traffic of visited websites
https://www.privacytechlab.org/
Other
28 stars 1 forks source link

addressed #552 #554

Closed danielgoldelman closed 11 months ago

danielgoldelman commented 11 months ago

Closes #552 on merge

JoeChampeau commented 11 months ago

Does this address all edge cases? The ".[2 character SLD].[ccTLD]" seems to cover most of the sites we expect to encounter, but a bit of cursory research is telling me that several countries maintain their own specific lists of SLDs under their respective ccTLDs that people can register domains under (i.e. France's [example].avocat.fr). It might be useful to implement some package like tldjs or psl that compares against some list of these super esoteric SLDs that's constantly being updated. I can look into that if necessary.

SebastianZimmeck commented 11 months ago

Good find, @JoeChampeau! Indeed, I think it would be good if you could implement such package (or otherwise find a way to address these edge cases).

danielgoldelman commented 11 months ago

@JoeChampeau why use psl.parse instead of .get? Seems to be doing the same thing... also we gotta address error handling

JoeChampeau commented 11 months ago

@danielgoldelman No particular reason behind the decision to use psl.parse, I just saw it got the job done and didn't look any further. We can change it if you want.

Good point on the erorr handling. The commit I just pushed displays the proper popup when you try to open it on the homepage, and for other instances (like localhost) the function just returns the empty string (same as if url is undefined). Should we create another dummy popup page that says something like "Privacy Pioneer is unable to analyze this page/scan this url" for cases like that?

SebastianZimmeck commented 11 months ago

Should we create another dummy popup page that says something like "Privacy Pioneer is unable to analyze this page/scan this url" for cases like that?

Yes, that is a good idea!

Here is what the popup currently displays when there is no URL in the URL bar. But that situation may be different than this one. Maybe, in this case, per your suggestion, just "Privacy Pioneer is unable to analyze this page."?

Screenshot 2024-01-05 at 11 13 06 AM
JoeChampeau commented 11 months ago

Here is what the popup currently displays when there is no URL in the URL bar. But that situation may be different than this one. Maybe, in this case, per your suggestion, just "Privacy Pioneer is unable to analyze this page."?

Yeah, that sounds good. I think it'd be valuable to draw a distinction between "no tracking currently detected" and "this isn't a valid page"

SebastianZimmeck commented 11 months ago

Sounds good. Feel free to go ahead with the merge, @JoeChampeau.