Move hardcoded exceptions rules to more specific rules and unify exception handling #45
Conversation
bbondy
force-pushed
the
quora-login
branch
5 times, most recently
from
4092588
to
a499a24
Compare
bbondy
changed the title
bbondy
force-pushed
the
quora-login
branch
8 times, most recently
from
623f9ff
to
5803848
Compare
The blocking is redundant based on our TP Disconnect library blocking. But it's needed in order to trigger a search for an exception filter. Above and beyond EasyList and EasyPrivacy, we do Disconnect blocking in our TP library. This allows login to quora using the Facebook button to work
bbondy
force-pushed
the
quora-login
branch
3 times, most recently
from
29b742b
to
5e69a26
Compare
bbondy
force-pushed
the
quora-login
branch
2 times, most recently
from
315d0eb
to
716455c
Compare
bbondy
force-pushed
the
quora-login
branch
2 times, most recently
from
ac6e242
to
5b023f6
Compare
bbondy
changed the title
brave-unbreak.txt
Outdated
| ||fbcdn.net$third-party,domain=~facebook.com | ||
| @@||staticxx.facebook.com/ | ||
| @@||xx.fbcdn.net/ | ||
| @@||www.facebook.com/plugins/post.php?href= |
There was a problem hiding this comment.
There are a few more URLs for Facebook embeds:
www.facebook.com/plugins/video.phpwww.facebook.com/plugins/comment_embed.phpwww.facebook.com/plugins/page.php
If you want to test them all at once, you an use the test page I created last week.
|
Should we also move these exceptions to |
bbondy
force-pushed
the
quora-login
branch
7 times, most recently
from
47d0be1
to
c409024
Compare
| @@||staticxx.facebook.com/ | ||
| @@||xx.fbcdn.net/ | ||
| @@||www.facebook.com/*/plugin | ||
| @@||www.facebook.com/plugins/ |
There was a problem hiding this comment.
If we unblock all of the plugins, then we'll let through the Like/Share buttons that @tomlowenthal wanted us to block in brave/brave-browser#3241:
www.facebook.com/plugins/like.phpwww.facebook.com/plugins/share_button.php
|
Looping in @tomlowenthal because we may not want all of the widgets from https://fmarier.github.io/brave-testing/social-widgets.html to be allowed by default. I think we definitely want Facebook videos to work for example, but maybe the LinkedIn Share/Follow social buttons aren't valuable enough to users warrant the tracking by Microsoft. |
|
@fmarier @tomlowenthal I don't want to have that discussion here since this task is scoped to be about moving things from hardcoded lists to a unified exception list that we can build off of. I'll remove the LinkedIn one and do it in another PR that we can only merge once the option to control it exists. |
|
Mainly I suggest we avoid such discussions because if we just follow the 3 step plan I outlined in the section "Summary of the plan" from brave/brave-browser#3475 then we can completely skip the discussions which have no clear way to decide what's best globally. |
|
Talked to @fmarier on Slack and he's ok merging this since it retains the existing functionality. Going ahead. LinkedIn was removed and will be PR'ed separately for inclusion once options exist. |
Related issue: brave/brave-browser#3475'
Related PR: brave/brave-core#1770
This is the first part of a series of work that will be done within the next few weeks.
It unifies exception handling in one place.
In a later PR, we will be adding metadata to filter rules, and exposing options to control those rules.
It will be opt-out options at first, and then a later iteration will try going to opt-in with UX to notify the user that things are broken.
See this issue for a description of the follow up work that will be done:
brave/brave-browser#3475
This is specifically for:
Note that this does not change existing functionality, it just moves a hardcoded c++ list to a more unified place where we can iterate on to add options to exclude them.
For a test plan, see the linked issue above.