add 3rd party fingerprinting block option #10181

diracdeltas · 2017-07-28T00:21:12Z

fix #9029

regular panel:

compact:

Submitter Checklist:

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Added/updated tests for this change (for new code or code which already has tests).
Ran git rebase -i to squash commits (if needed).
Tagged reviewers and labelled the pull request as needed.

Test Plan:

Reviewer Checklist:

Tests

Adequate test coverage exists to prevent regressions
Tests should be independent and work correctly when run individually or as a suite ref
New files have MPL2 license header

NejcZdovc · 2017-08-02T01:12:28Z

docs/state.md

@@ -69,6 +69,12 @@ AppStore
  cookieblockAll: {
    enabled: boolean // enable all cookie/referer blocking
  },
+  fingerprintingProtection: {


Please sort this alphabetically

luixxiul · 2017-08-02T19:38:09Z

less/switchControls.less

@@ -85,3 +85,6 @@
    }
  }
 }
+.pullRight {


I have a fix which is much more straightforward. please wait for a moment.

++ please feel free to push to this branch (but do not rebase or squash commits)

Done bcdfa30

luixxiul · 2017-08-02T20:21:00Z

CC @bradleyrichter for review:

We could align the switches to a left row like this:

Which one is better?

diracdeltas · 2017-08-02T21:03:12Z

the fp setting was right-aligned because that is where it was before (so people might be used to clicking in that area), but i don't have a strong opinion. actually i think 3 rows with 2 items each would look best.

luixxiul · 2017-08-03T02:11:11Z

that would be look like this:

diracdeltas · 2017-08-03T05:23:33Z

@luixxiul i like the last one the most because it matches with our existing placement

bradleyrichter · 2017-08-03T05:46:31Z

I'll need to try a different layout. Or we need to give up the wide view.

…

On Aug 2, 2017, at 10:23 PM, yan ***@***.***> wrote: @luixxiul i like the last one the most because it matches with our existing placement — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

luixxiul · 2017-08-03T10:08:59Z

@diracdeltas @bradleyrichter OK, so should we tag this PR as WIP for now?

diracdeltas · 2017-08-03T14:02:58Z

This is blocking some work that others are doing on improving fingerprinting protection, so I would prefer it not be pushed to a later milestone. Why can't we just do compact-panel only?

diracdeltas · 2017-08-04T15:30:43Z

Can i get this approved and then @bradleyrichter open up a follow-up issue for the UI tweak?

@cezaraugusto

Also: - Add globalStyles.appIcons.question Addresses #9029 Auditors: @cezaraugusto Test Plan: 1. Open about:preferences#shields 2. Enable compact panel 3. Open https://twitter.com 4. Open compact panel 5. Make sure the fp block pulldown is placed properly 6. Disable compact panel 7. Open twitter.com again 8. Open the default panel 9. Make sure the fp pulldown is placed properly

luixxiul · 2017-08-05T15:06:58Z

app/renderer/components/styles/global.js

@@ -241,21 +241,22 @@ const globalStyles = {
    prefsPanelHeading: '23px'
  },
  appIcons: {


elements below this are alphabetized.

luixxiul · 2017-08-05T15:08:51Z

app/renderer/components/main/braveryPanel.js

+                  styles.braveryPanel__body__advanced__control__forms__title
+                )}>
+                  <span data-l10n-id='fingerprintingProtection' />
+                  <span className={globalStyles.appIcons.question}


braveryPanel__body__advanced__control__fpWrapper is created to wrap the l10n string and the question mark to align them with flex.

Jacalz · 2017-08-05T15:18:11Z

From a users perspective I think this looks best:

This makes it possible to add two more things to the panel in the future, one with a drop down menu and also one with a switch 🙂

bsclifton · 2017-08-07T06:49:08Z

app/sessionStore.js

@@ -675,6 +675,28 @@ module.exports.runPreMigrations = (data) => {
 }

 module.exports.runPostMigrations = (data) => {
+  // fingerprinting protection migration


Not that we have to do it now... but I'm Curious what you think @diracdeltas and @darkdh

Similar to Ruby and Python, maybe we could create a /migrations/ folder and have a single file (ex: 0-20-0_1st_3rd_fingerprint) with a two possible functions (postMigrate, preMigrate) for each type of migration that we'd like to run.

We could then call them like so:

module.exports.runPostMigrations = (data) => { migrations = [ require('../migrations/0-12-0_search_xml'), require('../migrations/0-14-0_autofill_data'), require('../migrations/0-20-0_1st_3rd_fingerprint') ] migrations.runPostMigrations() }

I really like this idea ++

i'm for a migrations-migration. :)

We were talking with @bbondy when can we remove some old migrations from session store file and this solution would solve it. What I would suggest as well is to create an upgrade script. So it automatically check if file for that version exists or not. For example: let's say that you are upgrading from 0.10 to 0.15.4. When we start the upgrade process we determinate that we need to check versions 0.10.x to 0.15.4 (it can be many versions in between, hot fixes). Then we check if file migration/0.10.15.js (hot fix) exist and if so run both pre and post migration. Then you check if migration/0.11.jsexist and so on. I would have one file per version and it would be standardized with predefined functions. This way we know exactly what was added/removed in which version.

++ for aggregated migrations. We have many migration scattered around now and some of them are made by me 😝

diracdeltas · 2017-08-15T01:03:31Z

This has been updated to use @Jacalz's recommended layout. @bsclifton can you review?

bradleyrichter · 2017-08-15T01:10:14Z

I think what @Jacalz added above will be fine for now. I am adding another switch so I will re-examine the layout very soon.

@luixxiul Can you adjust the layout as shown above?

diracdeltas · 2017-08-15T01:14:41Z

@bradleyrichter see my comment above yours. no need to adjust since i already did.

luixxiul · 2017-08-15T01:14:47Z

Let me confirm the adjustment by @diracdeltas.

bradleyrichter · 2017-08-15T01:18:48Z

oops. sorry @diracdeltas. Thanks...

fix #9029

@cezaraugusto

Also: - Add globalStyles.appIcons.question Addresses #9029 Auditors: @cezaraugusto Test Plan: 1. Open about:preferences#shields 2. Enable compact panel 3. Open https://twitter.com 4. Open compact panel 5. Make sure the fp block pulldown is placed properly 6. Disable compact panel 7. Open twitter.com again 8. Open the default panel 9. Make sure the fp pulldown is placed properly

luixxiul · 2017-08-15T01:22:25Z

LGTM!

codecov-io · 2017-08-15T01:23:15Z

Codecov Report

Merging #10181 into master will decrease coverage by 0.06%.
The diff coverage is 43.18%.

@@            Coverage Diff             @@
##           master   #10181      +/-   ##
==========================================
- Coverage   54.35%   54.29%   -0.07%     
==========================================
  Files         245      245              
  Lines       21085    21117      +32     
  Branches     3249     3258       +9     
==========================================
+ Hits        11461    11465       +4     
- Misses       9624     9652      +28

Flag	Coverage Δ
#unittest	`54.29% <43.18%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
js/constants/settings.js	`100% <ø> (ø)`	⬆️
app/renderer/components/styles/global.js	`100% <ø> (ø)`	⬆️
js/constants/appConfig.js	`100% <ø> (ø)`	⬆️
js/state/syncUtil.js	`39.35% <0%> (-0.31%)`	⬇️
js/state/siteSettings.js	`91.39% <100%> (+0.66%)`	⬆️
js/about/preferences.js	`43.85% <15.38%> (-0.07%)`	⬇️
app/sessionStore.js	`62.74% <50%> (-2.37%)`	⬇️

bsclifton · 2017-08-15T05:08:28Z

I created #10488 so that we don't lose the ideas mentioned here 😄

bsclifton

Changes look great (implementation matches spec, blocking/unblocking fingerprint works great) 😄 Awesome job on the tests!

@diracdeltas can you add some manual tests steps (if required)? ex: a website that uses 3rd party fingerprinting. If it's covered by tests, let's tag this as QA/no-qa-needed 👍

BTW- do we need to update the wiki?
https://github.com/brave/browser-laptop/wiki/Fingerprinting-Protection-Mode

bsclifton · 2017-08-15T06:01:26Z

NOTE: I updated the original issue with a link to a jsfiddle where 3rd party fingerprint block can be tested:
https://jsfiddle.net/bkf50r8v/13/

@cezaraugusto

Also: - Add globalStyles.appIcons.question Addresses brave#9029 Auditors: @cezaraugusto Test Plan: 1. Open about:preferences#shields 2. Enable compact panel 3. Open https://twitter.com 4. Open compact panel 5. Make sure the fp block pulldown is placed properly 6. Disable compact panel 7. Open twitter.com again 8. Open the default panel 9. Make sure the fp pulldown is placed properly

diracdeltas added the PR/work-in-progress ⚒ label Jul 28, 2017

diracdeltas self-assigned this Jul 28, 2017

NejcZdovc reviewed Aug 2, 2017

View reviewed changes

diracdeltas force-pushed the feature/9029 branch from 23697f9 to 0dacae2 Compare August 2, 2017 07:06

diracdeltas removed the PR/work-in-progress ⚒ label Aug 2, 2017

diracdeltas added this to the 0.21.x (Nightly Channel) milestone Aug 2, 2017

diracdeltas requested review from NejcZdovc and luixxiul August 2, 2017 07:08

diracdeltas changed the title ~~WIP: add 3rd party fingerprinting block option~~ add 3rd party fingerprinting block option Aug 2, 2017

luixxiul added the feature/shields label Aug 2, 2017

luixxiul reviewed Aug 2, 2017

View reviewed changes

luixxiul added the needs-info Another team member needs information from the PR/issue opener. label Aug 3, 2017

diracdeltas removed the request for review from NejcZdovc August 4, 2017 17:21

diracdeltas added the PR/needs-rebase ۞ label Aug 4, 2017

diracdeltas force-pushed the feature/9029 branch from 0dacae2 to fb3f28e Compare August 4, 2017 22:36

luixxiul reviewed Aug 5, 2017

View reviewed changes

luixxiul removed the PR/needs-rebase ۞ label Aug 5, 2017

luixxiul requested a review from cezaraugusto August 5, 2017 15:09

bsclifton reviewed Aug 7, 2017

View reviewed changes

bsclifton mentioned this pull request Aug 7, 2017

Replace hideLower with showLess #10164

Merged

8 tasks

luixxiul added the PR/needs-rebase ۞ label Aug 9, 2017

diracdeltas force-pushed the feature/9029 branch from bcdfa30 to 93bd108 Compare August 15, 2017 01:00

diracdeltas removed needs-info Another team member needs information from the PR/issue opener. PR/needs-rebase ۞ labels Aug 15, 2017

diracdeltas and others added 3 commits August 15, 2017 01:22

add 3rd party fingerprinting block option

b8105cc

fix #9029

rearrange bravery panel layout

ce3a8ae

diracdeltas force-pushed the feature/9029 branch from 93bd108 to ce3a8ae Compare August 15, 2017 01:23

bsclifton mentioned this pull request Aug 15, 2017

Session migrations should be encapsulated and cleaned up #10488

Closed

bsclifton approved these changes Aug 15, 2017

View reviewed changes

bsclifton merged commit e8f32db into master Aug 15, 2017

bsclifton deleted the feature/9029 branch August 15, 2017 05:49

luixxiul mentioned this pull request Aug 15, 2017

Polish bravery panel #10489

Merged

8 tasks

luixxiul mentioned this pull request Aug 19, 2017

Speedtest.net Beta not loading #979

Closed

bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add 3rd party fingerprinting block option #10181

add 3rd party fingerprinting block option #10181

diracdeltas commented Jul 28, 2017 •

edited by bsclifton

Loading

NejcZdovc Aug 2, 2017

luixxiul Aug 2, 2017

diracdeltas Aug 3, 2017

luixxiul Aug 5, 2017

luixxiul commented Aug 2, 2017 •

edited

Loading

diracdeltas commented Aug 2, 2017

luixxiul commented Aug 3, 2017 •

edited

Loading

diracdeltas commented Aug 3, 2017

bradleyrichter commented Aug 3, 2017 via email

luixxiul commented Aug 3, 2017

diracdeltas commented Aug 3, 2017

diracdeltas commented Aug 4, 2017

luixxiul Aug 5, 2017

luixxiul Aug 5, 2017 •

edited

Loading

Jacalz commented Aug 5, 2017 •

edited

Loading

bsclifton Aug 7, 2017 •

edited

Loading

NejcZdovc Aug 7, 2017

diracdeltas Aug 7, 2017

NejcZdovc Aug 7, 2017

darkdh Aug 7, 2017

diracdeltas commented Aug 15, 2017

bradleyrichter commented Aug 15, 2017

diracdeltas commented Aug 15, 2017

luixxiul commented Aug 15, 2017

bradleyrichter commented Aug 15, 2017

luixxiul commented Aug 15, 2017

codecov-io commented Aug 15, 2017 •

edited

Loading

bsclifton commented Aug 15, 2017

bsclifton left a comment •

edited

Loading

bsclifton commented Aug 15, 2017 •

edited

Loading

add 3rd party fingerprinting block option #10181

add 3rd party fingerprinting block option #10181

Conversation

diracdeltas commented Jul 28, 2017 • edited by bsclifton Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luixxiul commented Aug 2, 2017 • edited Loading

diracdeltas commented Aug 2, 2017

luixxiul commented Aug 3, 2017 • edited Loading

diracdeltas commented Aug 3, 2017

bradleyrichter commented Aug 3, 2017 via email

luixxiul commented Aug 3, 2017

diracdeltas commented Aug 3, 2017

diracdeltas commented Aug 4, 2017

Choose a reason for hiding this comment

luixxiul Aug 5, 2017 • edited Loading

Choose a reason for hiding this comment

Jacalz commented Aug 5, 2017 • edited Loading

bsclifton Aug 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diracdeltas commented Aug 15, 2017

bradleyrichter commented Aug 15, 2017

diracdeltas commented Aug 15, 2017

luixxiul commented Aug 15, 2017

bradleyrichter commented Aug 15, 2017

luixxiul commented Aug 15, 2017

codecov-io commented Aug 15, 2017 • edited Loading

Codecov Report

bsclifton commented Aug 15, 2017

bsclifton left a comment • edited Loading

Choose a reason for hiding this comment

bsclifton commented Aug 15, 2017 • edited Loading

diracdeltas commented Jul 28, 2017 •

edited by bsclifton

Loading

luixxiul commented Aug 2, 2017 •

edited

Loading

luixxiul commented Aug 3, 2017 •

edited

Loading

luixxiul Aug 5, 2017 •

edited

Loading

Jacalz commented Aug 5, 2017 •

edited

Loading

bsclifton Aug 7, 2017 •

edited

Loading

codecov-io commented Aug 15, 2017 •

edited

Loading

bsclifton left a comment •

edited

Loading

bsclifton commented Aug 15, 2017 •

edited

Loading