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

Show an adblock admonition in the dev console #3894

Merged
merged 3 commits into from Apr 17, 2018

Conversation

Projects
None yet
4 participants
@davidfischer
Contributor

davidfischer commented Apr 3, 2018

This PR shows an adblock admonition in the browser developer console. While I don't think this will convert many hearts and minds, I believe it is a good first step.

screen shot 2018-04-02 at 10 24 10 pm

@davidfischer

This comment has been minimized.

Contributor

davidfischer commented Apr 3, 2018

To test this locally, change api_host in user_builds/<project>/rtd-builds/latest/_static/readthedocs-data.js to https://readthedocs.org and load those docs.

@davidfischer davidfischer requested a review from ericholscher Apr 3, 2018

@ericholscher

Seems simple enough. Doubt it will do much, but seems worth testing out.

console.error('Error loading Read the Docs promo');
if (xhr && xhr.status === 404 && rtd.api_host === 'https://readthedocs.org') {

This comment has been minimized.

@ericholscher

ericholscher Apr 4, 2018

Member

Feel like we probably need more of a check here. Is there an error thrown when it's blocked? In chrome I see "Blocked by the client" -- is there a specific error we should be looking for that is similar?

This comment has been minimized.

@davidfischer

davidfischer Apr 4, 2018

Contributor

From jQuery's perspective, it is just a 404.

@davidfischer

This comment has been minimized.

Contributor

davidfischer commented Apr 4, 2018

I don't think this will do anything with respect to getting people to whitelist us. However, it illustrates how to detect an ad blocker and simply logs to the console (which might alert us to any false positives) before we take any further action.

console.log(' - only show advertisements of interest to developers');
console.log('You can read more about our approach to advertising here: https://docs.readthedocs.io/en/latest/ethical-advertising.html');
console.log('%cPlease whitelist *.readthedocs.io on your adblocker!', 'font-size: 2em');

This comment has been minimized.

@humitos

humitos Apr 4, 2018

Member

I think we need to say *.readthedocs.org maybe?

Look at this

captura de pantalla_2018-04-04_12-07-37

This comment has been minimized.

@humitos

humitos Apr 4, 2018

Member

it comes from EasyList

This comment has been minimized.

@davidfischer

davidfischer Apr 4, 2018

Contributor

That's a good catch. I need to review this. We may need both...

This comment has been minimized.

@davidfischer

davidfischer Apr 4, 2018

Contributor

This is a larger problem than I thought.

Whitelisting *.readthedocs.io is all that is necessary (on AdblockPlus at least) to allow Read the Docs ads on all of our sites hosted on a subdomain. However, ads will continue to be blocked on any custom domains where we host ads. Whitelisting readthedocs.org does not change this. This makes simple whitelisting impossible across all ~10,000 domains where we host ads.

This comment has been minimized.

@davidfischer

davidfischer Apr 4, 2018

Contributor

After reviewing their filter syntax, we need the following two filters:

  • @@||readthedocs.org/* (exception for making an ad request)
  • @@readthedocs.io^$document (exception for hiding elements)
@davidfischer

This comment has been minimized.

Contributor

davidfischer commented Apr 4, 2018

I noted the specific commands needed to whitelist across our custom domains.

@agjohnson agjohnson added this to the 2.4 milestone Apr 17, 2018

@davidfischer davidfischer merged commit 5c9fd18 into rtfd:master Apr 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment