Skip to content
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

Add options for controlling how blocking is done at a high level #3489

Closed
bbondy opened this issue Feb 25, 2019 · 2 comments · Fixed by brave/brave-core#1818
Closed

Add options for controlling how blocking is done at a high level #3489

bbondy opened this issue Feb 25, 2019 · 2 comments · Fixed by brave/brave-core#1818
Assignees
Labels
priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include

Comments

@bbondy
Copy link
Member

bbondy commented Feb 25, 2019

This is Step 2 of the plan outlined in this issue:
#3475
In the section "Summary of the plan"

It looks like this:
Screen Shot 2019-03-10 at 11 11 09 PM

Step 2: Add options to easily disable the exception rules, it will be allowed by default when this step lands as it is today for FB login buttons, embedded tweets, and embedded FB posts.
This will allow you to uncheck things, and it will look like:

Test Plan

Before testing on this can be done, the data-files must be updated from:
brave/adblock-lists#54
If that pull request is closed, then you can assume they are updated.

  1. Make sure defaults look like this:
* [x]  Allow Google login buttons on third party sites
* [x]  Allow Facebook logins and embedded posts
* [x]  Allow Twitter embedded tweets
* [ ]  Allow LinkedIn embeds
  1. Use this test page: https://fmarier.github.io/brave-testing/social-widgets.html and make sure toggling settings and reloading the page works correctly.

Note that between reloads you need to do a clean reload (cmd+shift+r) so that it doesn't use cache.

  1. Test logins to the following sites work:
    https://twitch.tv FB login
    https://quora.com FB login
    https://www.espn.com/login/ FB login
    https://www.etsy.com/signin FB login

  2. Retest these issues about embedded tweets, they should still work.
    Image from facebook/twitter not loading browser-laptop#2014
    Embeded Tweets browser-laptop#1208

  3. Test that Google login only works when the option is on for https://pinterest.com/login/
    You will need to reload between login attempts after the setting is changed.
    You may need to click the login button twice, if so the issue is not related to this ticket but can be posted.

@tildelowengrimm
Copy link
Contributor

cc: @karenkliu

@srirambv
Copy link
Contributor

srirambv commented Mar 21, 2019

Verification passed on

Brave 0.62.37 Chromium: 73.0.3683.86 (Official Build) beta (64-bit)
Revision f9b0bec6063ea50ce2b71f5b9abbae7beee319a6-refs/branch-heads/3683@{#858}
OS Linux

Verification passed on

Brave 0.62.40 Chromium: 73.0.3683.86 (Official Build) beta (64-bit)
Revision f9b0bec6063ea50ce2b71f5b9abbae7beee319a6-refs/branch-heads/3683@{#858}
OS Windows 10 OS Build 17134.523

Verification passed on

Brave 0.63.20 Chromium: 73.0.3683.75 (Official Build) dev (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Windows 10 OS Build 17134.523

Used test plan from the description

Verified passed with macOS 10.13.6 using

Brave 0.62.49 Chromium: 73.0.3683.86 (Official Build) (64-bit)
Revision f9b0bec6063ea50ce2b71f5b9abbae7beee319a6-refs/branch-heads/3683@{#858}
OS Mac OS X
  • Verified test plan from description
  • Verified working similar to disconnect.me

Verification PASSED on macOS 10.14.3 x64 using the following build:

Brave 0.62.50 Chromium: 73.0.3683.86 (Official Build) (64-bit)
Revision f9b0bec6063ea50ce2b71f5b9abbae7beee319a6-refs/branch-heads/3683@{#858}
OS Mac OS X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants