pixeltris / TwitchAdSolutions

8.08k stars 461 forks source link

Rolling out Twitch adblocking in Brave #282

Open ryanbr opened 2 months ago

ryanbr commented 2 months ago

Just a heads up,

In the process of rolling out twitch adblocking https://github.com/brave/brave-core/pull/25546

Using https://github.com/brave/adblock-lists/blob/master/brave-lists/brave-twitch.txt (Scriplet: https://github.com/brave/adblock-resources/blob/master/resources/vaft-ublock-origin.js)

I haven't seen any reports of broken issues other than https://old.reddit.com/r/brave_browser/comments/1f7ny6s/twitchtv_doesnt_work_anymore/ But I don't believe its an issue with the scriplet here.

Want to test it to be sure it works well? Enable Brave experimental Adblock Rules in brave://settings/shields/filters

Does it work well?

kodiakhub commented 2 months ago

From TR, it’s working well.

  1. Installed fresh Brave. (nothing changed, default)
  2. Only enabled Brave exprimental Adblock Rules
  3. Navigated a lot of streams without login
  4. No issues found about the prerolls and midrolls ads

Just in case, some extension/userscript may cause the Twitch Adblock method to breakage.

For example, FrankerFaceZ Addons:

image

I noticed that this option caused the issue when I used AdGuard Extra. This Addon setting was causing the midrolls infinite buffering time to time. When I disabled this setting, no buffering problem since then. I won't be able to check that for Brave, because it's diffucult to trigger midrolls. So just to inform. 👍

stevenya97 commented 2 months ago

Likely conflicting with AdGuard Extra's vaft script. Vaft will switch to a lower quality stream with no ads during midrolls, as the normal stream will have the full screen ad, and switch back once midroll is over. Potentially this addon setting is breaking this core function and causing it to buffer. Would have to look at the code to be sure, but it not happening with the setting disable certainly suggests so. Could be something to note in this repo's README down the line if more people come across the same issue.

kodiakhub commented 2 months ago

Could be something to note in this repo's README down the line if more people come across the same issue.

Looks like there is a similar issue related on this PR, see: https://github.com/pixeltris/TwitchAdSolutions/pull/199

I was manually solving the infinite buffering issue with pause/play.

ryanbr commented 2 months ago

@pixeltris is there a downside to https://github.com/pixeltris/TwitchAdSolutions/pull/199 ?

ryanbr commented 2 months ago

@kodiakhub I don't think we can avoid issues with 3rd party twitch scripts/extensions. Similar to the issues people see with Youtube ads, list authors don't recommend using privacy/YT/other adblock extensions which may trigger false positives.

kodiakhub commented 2 months ago

@kodiakhub I don't think we can avoid issues with 3rd party twitch scripts/extensions. Similar to the issues people see with Youtube ads, list authors don't recommend using privacy/YT/other adblock extensions which may trigger false positives.

It's understandable, at least might think consider adding a Tooltips about 3rd party extensions and userscripts etc at brave://settings/shields/filters. Similar to "Note that enabling too many filters will degrade browsing speeds."

It may be useful like for end users.

ryanbr commented 2 months ago

We'll take it on board, though users probably won't notice it unless its pointed out (via a support agent)

ryanbr commented 2 months ago

@pixeltris Getting some reports; ( a few hundred)

video player not loading. applies to every stream i visit
Twitch broadcasts are not opening, I tried to delete cookies again.
twitch live videos/vods doesn't work with brave Shields on for some reason
twitch won't play (took forever to load) the video without disabling shield
ryanbr commented 2 months ago

Trying to reach to one user, in case its an extension compatibility issue.

kodiakhub commented 2 months ago
video player not loading. applies to every stream i visit
Twitch broadcasts are not opening, I tried to delete cookies again.
twitch live videos/vods doesn't work with brave Shields on for some reason
twitch won't play (took forever to load) the video without disabling shield

I'm wondering something, do these report complaints go through a process of prerequisites etc. (i.e. related to the 3rd party issue)? 🤔

ryanbr commented 2 months ago

https://old.reddit.com/r/brave_browser/comments/1fjqmd8/brave_shield_no_longer_working_on_twitch/ https://old.reddit.com/r/brave_browser/comments/1fjrhxl/brave_twitch_adblock_rules_list_has_started_to/

Asking about extensions, my hunch

kodiakhub commented 2 months ago

https://old.reddit.com/r/brave_browser/comments/1fjqmd8/brave_shield_no_longer_working_on_twitch/ https://old.reddit.com/r/brave_browser/comments/1fjrhxl/brave_twitch_adblock_rules_list_has_started_to/

Asking about extensions, my hunch

In my opinion, processes like this only increase the workload and waste time. Due to XY problem. To put it simply, the uBO reddit has come a long way in this regard. (Guide, Rules etc.) I think similar system is needed for Brave like AdGuard and uBO prerequisite systems. I'm just saying. 👍

kodiakhub commented 2 months ago

@ryanbr You can see the situation by looking at the Brave reddit comments, right? 🙂

pixeltris commented 2 months ago

No playback / infinite loading wheel means conflicting solutions (Worker hooked multiple times). There's a list of chromium extensions here which all use an older copy of vaft https://github.com/pixeltris/TwitchAdSolutions/blob/master/full-list.md#extensions-to-avoid. Similarly @arthurbolsoni's Purple Adblock. Someone could also have an old vaft / video-swap-new permalink hooked up to Brave which would conflict. There probably are more.

@pixeltris is there a downside to https://github.com/pixeltris/TwitchAdSolutions/pull/199 ?

This PR is specifically for improving freezing issues which is a separate thing from the no playback / infinite loading wheel. The Worker hook check could probably be improved such as preventing overwriting (i.e. the reverse of #169), though this may cause other problems.

In terms of #199 as a way to fix freezing; I haven't been able to test much. On chromium I didn't experience the freezing often and midrolls need to be tested as that's where most of freezing issues occur on chromium.

can’t use the redeem function or whispers or even subscribe to people. I moderate a community as well and I can’t do any of the mod actions

I'm not too sure about this one as I haven't tested anything with an account in a long time. There's a similar thing with clip creation #121

ShivanKaul commented 2 months ago

Is there any way we can detect in the JS that there's an older copy of vaft being used, and in that case, not inject or try to register the Worker?

pixeltris commented 2 months ago

The new script already has such a check. It works if all scripts have the check or if the old script loads before the new script. But it doesn't help when the old script loads after the new script

https://github.com/pixeltris/TwitchAdSolutions/blob/859802410009e3a9bf47a804a502f2dbbf38d150/vaft/vaft.user.js#L869

Object.defineProperty with writable:false as a way of assigning the Worker seems to work when the old script loads after the new script. But it feels like a bad fix. If another script comes along which also uses Object.defineProperty as assignment and doesn't check the integrity of the Worker you'll run into the same problems.

ShivanKaul commented 2 months ago

If another script comes along which also uses Object.defineProperty as assignment and doesn't check the integrity of the Worker you'll run into the same problems.

I think at some point we'll just have to tell users that they shouldn't be installing janky extensions. It's the first roll out that's the tricky thing.

ShivanKaul commented 2 months ago

If a user installs an extension that immediately starts causing issues, then they know that it's probably the new extension that's at fault. Currently however, given that we're deploying this new script out, it looks like TwitchAdSolutions/vaft/vaft.user.js is breaking things, not the older and worse scripts.

ryanbr commented 2 months ago

Just to outline we reverted the twitch scriptlet due to the breakages from other extensions, When we test this I had no issues it did a good job at block the adverts.

But I wasn't using any 3rd party twitch extensions either. Having the script exit gracefully if detecting an extension or other methods would be ideal tbh. From what was discussed from the community these 2 extensions came up:

pixeltris commented 2 months ago

As mentioned above using Object.defineProperty / writable:false to assign the Worker is the only way I can see to prevent the double Worker hook with the current versions of the extensions. I can't say what side effects that may have, but it does seem to work from my limited testing.

Alternatively you'd need to do something on the Brave side of things to check for installed extensions as the script doesn't have access to this kind of information.

pixeltris commented 1 month ago

https://github.com/pixeltris/TwitchAdSolutions/commit/69f098d882eba30a6466ad99eadf487c0aa68139 should fix it. This checks if other scripts / extensions have modified the Worker after the script does. With the current versions of Stream Cleaner / Purple Ads Blocker it'll always use their extension rather than the script.

ryanbr commented 1 month ago

Asking if the user has any extensions installed; Not sure if you've heard of this issue.

https://x.com/CryptoWaDummy/status/1839879698509074786

I was using the Content filter "Brave Twitch Adblock rules" while I was in a streamers chat it would say it was in emote only. In reality it was not. When I disabled the filter it worked as intended

ryanbr commented 1 week ago

https://github.com/brave/brave-browser/issues/22370#issuecomment-2464279890

Some initial testing with various extensions (old and new)

pixeltris commented 1 week ago

I see with the recent change https://github.com/pixeltris/TwitchAdSolutions/commit/722f5b96241ef1d92998b87f941690e47e475379 there's a regression with the script + TTV LOL PRO which results in neither being applied. @younesaassila

Stream Cleaner does work from my testing, both alone and with the experimental Brave filter.

Better Twitch Adblock / Adblock for Twitch are broken as you mention as they are outdated / lack this fix https://github.com/pixeltris/TwitchAdSolutions/commit/044d1fb3bb88a19ab718603069354d43eeba7015. But it's still triggering the Brave script's check for another active solution so ads get through.

Not ideal. I'll have a think about the conflicts with broken solutions.

ryanbr commented 1 week ago

Its better than breaking the site to be fair (the endless black screen looping issues previously), but would be nice if interactions between TwitchAdSolutions and some extensions could be tweaked/improved.

Thanks for updates and contributions. Much appreciated

pixeltris commented 1 week ago

Try out this version https://gist.github.com/pixeltris/8c40313fcc8da2687381ba7b6c10adff (diff: https://gist.github.com/pixeltris/8c40313fcc8da2687381ba7b6c10adff/revisions)

It should always block ads and there shouldn't be conflicts with the extensions you tested.

It has the following properties:

Problems:

ryanbr commented 1 week ago

Nice. can you update the *-ublock-origin.js ? How safe is it to roll out for testing in Brave?

pixeltris commented 1 week ago

Yea, when I get the chance I'll fix the possible issues with the message handling and push it to the repo once I've tested it properly.