gorhill / uBlock

uBlock Origin - An efficient blocker for Chromium and Firefox. Fast and lean.
GNU General Public License v3.0
47.53k stars 3.16k forks source link

Add Anti-Adblock Killer Continued #2685

Closed comfyneet closed 7 years ago

comfyneet commented 7 years ago

It seems that reek's AAK repo has been discontinued/abandoned for over 7 months (reek/anti-adblock-killer#3272), some features don't work any longer. Fortunately, someone else forks and maintains this repo at GitLab.

I think you should consider to add this new filter to the "3rd-party filters" tab.

jspenguin2017 commented 7 years ago

Sounds good. The link to the filter is here: https://xuhaiyang1234.gitlab.io/AAK-Cont/dist/aak-cont-list-ubo.txt. It's an old version for now, but hopefully I'll have time to update it soon.

The build pipeline for AAK-Cont is choked up for now, I have more renaming planned for uBlock Protector so I'll fix AAK-Cont build scripts later. This is probably going to solve Userscript integration issues AAK-Cont currently suffers. But it might cause more confusion in how to properly install AAK-Cont, @Giwayume what's your opinion on this? Should the filter list be fully stand-alone? And have another filter list + Userscript in case the user wants more?

The Userscript is a bit out of sync too, @Giwayume do you have any plan on automating script rule porting?

Giwayume commented 7 years ago

I'd like to keep it to one filter list. This is pretty much the same situation that the old AAK had, uBO included the AAK filter list as an option upon installation, but there's the caveat that you need to install the script to get the full functionality. Ideally, a filter list should work on its own, but the whole point behind AAK is sometimes they don't.

My thought with the userscript is to write a node script to detect which filters need to be ported over/updated. The porting would still be a manual process, since you have some functions that only work in extension code.

I don't know when I'll get to that, though, since it's the end of the budget year I'm busy & with overtime at my job. My company likes to wait until last minute to fit all the projects in for the year for some reason.

jspenguin2017 commented 7 years ago

It's quite rare that I need to use privileged extension API, the main problems are:

I do have a few solutions that uses chrome.webRequest API, probably have to white list in the case of AAK-Cont. Also not sure what to do with IMA SDK surrogate.

jspenguin2017 commented 7 years ago

@Giwayume I'm not sure why not many people want to help. I think it is because Merge Requests are too much work and it's a bit hard to satisfy my filter and code quality standard. I'll relax the rules until at least 20 developers join us. Filter and script rules are accepted if they fixes more than they breaks. Write access is granted if requested in an Issue or a Merge Request. Any objection on this?

Giwayume commented 7 years ago

From the issues I've seen it's people who aren't confident enough in their Javascript abilities who said they want to help. I'm not sure how to solve the lack of contributor problem. I've certainly seen them trip up on which files to touch. Maybe the contributing guide needs a step by step tutorial.

joey04 commented 7 years ago

I've not used AAK or uBO protector, but looking at this thread and the filter link for the first time, here's a first impression critique that may be of help in maintaining your project and getting others to help support it:

1) Add whitespace and headers to organize it. When opening it for the first time I see a big wall of uninviting text. Other than the main header there's no section commenting. Something like EasyList may be a helpful guide in that regard, and would help keep it organized. Whitespace '!' or '#' separators here and there could help too (presumably added by your build scripts).

2) Rule consolidation. There's a big block of 40 rules of ||domain/visitScript/$script,third-party that could be consolidated into a single /visitScript/*$script,third-party,domain=a|b|c|etc rule. That's the way EasyList and others do it.

jspenguin2017 commented 7 years ago

@joey04 You are looking at the compiled and minimized file, white space and comments are intentionally removed. I know there are some confusion about which file is source which file is "binary", I have plans on updating the contributor guide. There are a lot of rules copied form the original AAK, I didn't try to beautify them in the fear of breaking things. The way to combine the rules you shown does change its behavior, as domain/notVisitScript/visitScript/ will be matched in the combined filter.

I am open to suggestions, but they need to be specific. Please submit a Merge Request with your suggested changes, then we can discuss about it.