Issue 29773577: Fixes #68 - Implemented address masking script (Closed)
|
Created:
May 7, 2018, 5:49 p.m. by juliandoucette Modified:
May 15, 2018, 3:12 p.m. Reviewers:
ire Base URL:
https://hg.adblockplus.org/website-defaults Visibility:
Public. |
DescriptionFixes https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/68
Patch Set 1 #
Total comments: 9
Patch Set 2 : Addressed #3 #
MessagesTotal messages: 8
heyo, I don't remember if we switched to using gitlab's website-defaults as a master repository yet. If we did, sorry about this. I can upload it there if you prefer.
Thanks Julian! I can't find the issue here, [this link](https://gitlab.com/eyeo/websites/website-defaults/issues/68) doesn't lead anywhere. On 2018/05/07 17:51:45, juliandoucette wrote: > heyo, I don't remember if we switched to using gitlab's website-defaults as a > master repository yet. If we did, sorry about this. I can upload it there if you > prefer. I don't think we have switched over yet so this is fine. https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... File pages/address-masking.md (right): https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... pages/address-masking.md:11: // 1. Base64 encoded sensitive address NIT: Why not separate the code blocks so the titles/steps can be outside the block. e.g. ### 1. Base64 encoded sensitive address ``` btoa("mailto:username@example.com") ``` ### 2. Place encoded address inside `data-address` ... https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... pages/address-masking.md:32: <a data-address="bWFpbHRvOnVzZXJuYW1lQGV4YW1wbGUuY29t">Private Email</a> What about when there is no JS? This makes the link unusable :( https://codereview.adblockplus.org/29773577/diff/29773578/static/js/address-m... File static/js/address-masking.js (right): https://codereview.adblockplus.org/29773577/diff/29773578/static/js/address-m... static/js/address-masking.js:23: target.href = atob(href); Suggest: Remove the data-address since it's no longer needed?
> I can't find the issue here, [this link](https://gitlab.com/eyeo/websites/website-defaults/issues/68) doesn't lead anywhere. It's https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/68 (sorry - I should have mentioned). https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... File pages/address-masking.md (right): https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... pages/address-masking.md:11: // 1. Base64 encoded sensitive address On 2018/05/07 23:02:23, ire wrote: > NIT: Why not separate the code blocks so the titles/steps can be outside the > block. e.g. > > ### 1. Base64 encoded sensitive address > > ``` > btoa("mailto:username@example.com") > ``` > > ### 2. Place encoded address inside `data-address` > > ... Done. https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... pages/address-masking.md:32: <a data-address="bWFpbHRvOnVzZXJuYW1lQGV4YW1wbGUuY29t">Private Email</a> On 2018/05/07 23:02:23, ire wrote: > What about when there is no JS? This makes the link unusable :( Yep :( ... I don't see a Frontend way around that. https://codereview.adblockplus.org/29773577/diff/29773578/static/js/address-m... File static/js/address-masking.js (right): https://codereview.adblockplus.org/29773577/diff/29773578/static/js/address-m... static/js/address-masking.js:23: target.href = atob(href); On 2018/05/07 23:02:23, ire wrote: > Suggest: Remove the data-address since it's no longer needed? Done.
Thanks Julian! I just have one question/reservation you should ask legal about. If legal is alright with this then your implementation LGTM https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... File pages/address-masking.md (right): https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... pages/address-masking.md:32: <a data-address="bWFpbHRvOnVzZXJuYW1lQGV4YW1wbGUuY29t">Private Email</a> On 2018/05/07 23:51:39, juliandoucette wrote: > On 2018/05/07 23:02:23, ire wrote: > > What about when there is no JS? This makes the link unusable :( > > Yep :( ... I don't see a Frontend way around that. Yeah, I'm not sure of a solution to this either. Can you check with legal that we can do this? Because this will make the email inaccessible to some percentage of users and that may not be an option.
Thanks Ire! Detail: I also tested printing in Firefox and Chrome. Works. https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... File pages/address-masking.md (right): https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... pages/address-masking.md:32: <a data-address="bWFpbHRvOnVzZXJuYW1lQGV4YW1wbGUuY29t">Private Email</a> On 2018/05/09 05:19:54, ire wrote: > Yeah, I'm not sure of a solution to this either. Can you check with legal that > we can do this? Because this will make the email inaccessible to some percentage > of users and that may not be an option. <julian> judith: Is it a problem if our phone numbers and email addresses are inaccessible to dumb robots and web browsers with JavaScript disabled? (for the sake of spam protection) (e.g. on the privacy policy page) <@judith> julian: for dumb robots defintely no problem at all. In terms of JavaScript disabled: how likely is that from your experience? <julian> judith: very very unlikely. JavaScript is enabled by default and most of the web doesn't work without it. There are two good reasons to disable JavaScript that I know of. 1. To test a worst case scenario in a website or webapp. 2. To protect your privacy. Most people protect their privacy using extensions like ours (that block known trackers) or privacy badger (which uses some additional privacy protecting heuristics). Those who take their privacy more seriously use extensions like NoScript which selectively blocks JavaScript using heuristics and explicitly asks for permission to run scripts and/or parts of scripts. Disabling JavaScript completely is usually an advanced setting. The only browser that I know of that places this setting in the forefront is the Tor Browser. <@judith> julian: ok, great. Thanks for the background info. Thus, no issue with javascript disabled either <julian> judith: 👍🏻 Thanks! <@judith> julian: thank you
Message was sent while issue was closed.
https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... File pages/address-masking.md (right): https://codereview.adblockplus.org/29773577/diff/29773578/pages/address-maski... pages/address-masking.md:32: <a data-address="bWFpbHRvOnVzZXJuYW1lQGV4YW1wbGUuY29t">Private Email</a> On 2018/05/15 09:53:36, juliandoucette wrote: > On 2018/05/09 05:19:54, ire wrote: > > Yeah, I'm not sure of a solution to this either. Can you check with legal that > > we can do this? Because this will make the email inaccessible to some > percentage > > of users and that may not be an option. > > <julian> judith: Is it a problem if our phone numbers and email addresses are > inaccessible to dumb robots and web browsers with JavaScript disabled? (for the > sake of spam protection) (e.g. on the privacy policy page) > > <@judith> julian: for dumb robots defintely no problem at all. In terms of > JavaScript disabled: how likely is that from your experience? > > <julian> judith: very very unlikely. JavaScript is enabled by default and most > of the web doesn't work without it. There are two good reasons to disable > JavaScript that I know of. 1. To test a worst case scenario in a website or > webapp. 2. To protect your privacy. Most people protect their privacy using > extensions like ours (that block known trackers) or privacy badger (which uses > some additional privacy protecting heuristics). Those who take their privacy > more seriously use extensions like NoScript which selectively blocks JavaScript > using heuristics and explicitly asks for permission to run scripts and/or parts > of scripts. Disabling JavaScript completely is usually an advanced setting. The > only browser that I know of that places this setting in the forefront is the Tor > Browser. > > <@judith> julian: ok, great. Thanks for the background info. Thus, no issue with > javascript disabled either > > <julian> judith: 👍🏻 Thanks! > > <@judith> julian: thank you A reason for no-js you didn't mention was performance, e.g. people using browsers like Opera Mini. However, based on our browser stats that may not be a significant enough portion of our users to think about. Which begs the question, why do we support no-js in our websites at all? 
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
