Page MenuHomePhabricator

Show notice on Pageviews Analysis for people with adblocker
Closed, ResolvedPublic5 Story Points

Description

When a user visits Pageview Analysis and can't see the chart because of an adblocker, we should show a notice that explains what's happening, and what they can do to see the chart.

Notice text TBD.

Event Timeline

DannyH created this task.Mar 29 2016, 5:13 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 29 2016, 5:13 PM

We have this, but I guess we could make it more pronounced:

We have this, but I guess we could make it more pronounced:

Can we avoid showing the html completely? Is there a way to detect that an ad-blocker is enabled?

MusikAnimal added a comment.EditedMar 29 2016, 5:46 PM

Can we avoid showing the html completely? Is there a way to detect that an ad-blocker is enabled?

Good thinking. We could do something similar to http://stackoverflow.com/a/20505898/604142

Since our JS/CSS is under /pageviews, we'd have to use obtrusive JS to remove the other markup. So first, in say /javascripts/pv.js (since it's shared with our Topviews app), we could put window.noAdBlockers = true

Then in footer.haml (since it's also a shared file):

:javascript
  if(!window.noAdBlockers) {
    // show notice
  }

If /pageviews/application.js (where pv.js is concatenated to) successfully loads, the global variable is set to true and that above code is never ran.

This won't definitively assert the API itself is blocked, say if /metrics were also blocked, but from what we know it's only /pageviews that is the issue.

DannyH set the point value for this task to 5.Mar 29 2016, 5:52 PM

See https://github.com/MusikAnimal/pageviews/issues/48

I guess we need to also say that cookies must be allowed for tools.wmflabs.org. I thought this was because of Intuition, as it's what is adding cookies, but I see that if I have cookies disabled the content is still translated. I will try to do more investigation.

Either way we should probably at least encourage allowing cookies for translation purposes. Additionally I think we're required to have some notice of cookie usage for European users under the EU cookie directive.

I guess we need to also say that cookies must be allowed for tools.wmflabs.org. I thought this was because of Intuition, as it's what is adding cookies, but I see that if I have cookies disabled the content is still translated. I will try to do more investigation.

I don't see the chart at all with cookies disabled (testing in an incognito tab), without any adblockers enabled.

MusikAnimal added a comment.EditedApr 1 2016, 3:56 PM

I don't see the chart at all with cookies disabled (testing in an incognito tab), without any adblockers enabled.

Indeed, which I'm a bit confused by. We certainly don't need cookies to query the RESTBase API, so my guess is the lack of cookie support causes a PHP error in Intuition, and affects rendering of the rest of the page.

Fortunately we are able to reliably detect if cookies are disabled, see this StackOverflow. If they are we can show a message telling the user they need to allow cookies tools.wmflabs.org. What's even more fortunate – Intution still checks the headers and will set the language to whatever the user's browser/computer is set to, meaning we can allow for translations of the cookie message even though the language preference can't be set (because cookies are disabled).

Hopefully that makes sense. Anyway I've got a plan for implementation, which I'm happy to take of. I'll put up a demo on pageviews-test and maybe you could give some feedback on the copy.

Update, see https://github.com/MusikAnimal/pageviews/issues/52

This user is right. XTools and other tools that use Intuition still function when cookies are disabled. Still trying to figure out why it's breaking for us. Aside from the bug fix, we'll also want to do as XTools does and still allow users to change the language. So if cookies are disabled, rather than setting the cookie in our JS, we'll refresh the page adding a URL parameter use_lang=en-US, etc.

kaldari moved this task from Ready to In Development on the Community-Tech-Sprint board.
kaldari added a subscriber: kaldari.Apr 7 2016, 1:07 AM

Is that what XTools does? refresh the page with use_lang=en-US? Seems kind of janky :)

Ha, well I'm certainly okay with lang=en-US or something similar. I just like the fallback of using query (in our case hash) parameters to allow users to change the language.

The cookies issue should be fixed now, per https://github.com/MusikAnimal/pageviews/pull/65 (at least as soon as that's merged and deployed).

@MusikAnimal: Are you still working on this task or is it finished?

@kaldari I can get this out tonight! I sidestepped it because of the cookies issue, which you've since resolved. Just need to remove that notice and we'll be all set

Try it out at https://tools.wmflabs.org/pageviews-test

This hides all other content from view, giving instructions on how to whitelist tools.wmflabs.org. We might want to look into i18n for this, not sure.

This has been merged and deployed. However, it seems my uBlock extension for Safari has auto-updated, as did AdBlock Plus on Firefox. Both are no longer blocking the Pageviews app or the API queries :)

Looking at easyprivacy.txt, I see only tools.wmflabs.org is whitelisted, and not the RESTBase API. I guess given the base domain of our app is whitelisted, it whitelists everything else the app calls, too. Perhaps others' ad blockers are auto-updating as well.

kaldari closed this task as Resolved.Apr 19 2016, 5:42 AM

Well that's good news!

DannyH moved this task from Estimated to Archive on the Community-Tech board.May 3 2016, 4:20 PM