addressed #552 #554
addressed #552 #554
Conversation
danielgoldelman
added
small
|
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. |
|
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). |
|
@JoeChampeau why use psl.parse instead of .get? Seems to be doing the same thing... also we gotta address error handling |
|
@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? |
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."? 
|
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" |
|
Sounds good. Feel free to go ahead with the merge, @JoeChampeau. |
Closes #552 on merge