-
Notifications
You must be signed in to change notification settings - Fork 172
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
"--noads" option does not appear to work #47
Comments
It's an issue with WebPageTest itself and not the API wrapper. The ads blocking list hadn't been maintained/updated for a couple of years so it was removed. It might be worth updating but explicitly selecting the content you'd like to block tends to work better. |
Ok, I see. This seems like a really powerful feature if maintained. In my case, because I work on sites that have ads which may load something different each time the page is loaded ... each ad having different performance characteristics ... it's very valuable to be able to have an ad-less baseline of production performance vs with ads. That said, sending a block list manually seems fine too. Given the following block list https://easylist-downloads.adblockplus.org/easylist.txt how should I format that so that WPT understands it? Also, if you have a better idea than simply grabbing this list (and caching to disk/memory for say 24 hours) and cleaning up the format, I'd love any advice. |
I think it makes sense to remove this option from the list: #48 |
Do your sites use their own ad-loader that then goes out and decides what ad to load or is it a back-end decision and injected directly into the pages? Usually you just have to block the few root ad calls which then block everything downstream. The WPT blocking doesn't support everything that the adblock list supports (regex, DOM nodes, CSS selectors, etc). It only supports substring matches on the URLs. If you pull out just the substring URLs then you can use that as a block list (space separated) though there's a chance that it will add measurable overhead to scan that long of a list for every network request. |
We do in fact use a single loader. The url is always the same to a single item block list should work perfectly. Thanks for the idea! |
The block list method works well in my case. So I'm happy. I would however really like to encourage you (or the maintainer) to accept my pull request to remove the advertised "--no-ads" option from the README. That should help to prevent similar confusion. |
Yep, sorry, I don't have permissions here - I just run the WebPageTest side of things (and it was removed from the API docs so I think we're good over there). Marcel, if you get a chance can you look at the PR to remove the noads support? |
Thanks for your work on this is been very helpful. I have some questions / possible bug reports.
Here you can see I've submitted two tests with the cli. One with the
--noads
option and one without. But both results clearly render an Apple ad. In comparison, I'm running Chrome with the AdBlock Plus plugin and the ad is being blocked as expected. So if the option is suppose to be using the AdBlock Plus block list, I think this should work.Test (works for test command only)
for all of the options in which the --noads option is listed. I assume this means it should also work for the node library.runtTest(<someurl>, { noads: true|false }, ...)
equivalent as well ?Block Requests Containing...
option?This library has some pretty excellent CI and appears to be passing all tests. If this is a bug, perhaps in addition to fixing it we could add some coverage for this?

Let me know if I can lend a hand.
The text was updated successfully, but these errors were encountered: