Skip to content

Fix Web GUI breaking with ad blockers #560

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

Merged
merged 4 commits into from
Mar 7, 2022
Merged

Conversation

andocz
Copy link
Contributor

@andocz andocz commented Feb 24, 2022

Ad blockers using the EasyPrivacy filter list (e.g. uBlock Origin with the default configuration) block /scripts/ga.js from loading. When this happens, gtag never gets defined, which breaks much of the Web GUI's code, as it's full of if (gtag) { statements.

As a result, features like Git Blame and the diagrams don't work at all. This can be currently seen on codecompass.net's demo - Uncaught ReferenceError: gtag is not defined appears in the log when trying to use these features.
The demo on zolix.hu doesn't have the issue as it seems to run a version before Google Analytics was added.

Changing ga.js to have a less conspicuous name fixes the issue. Here ga-loader.js was used. In addition, ga-loader.js was changed to fail cleanly with an error message when googletagmanager.com cannot be reached (that domain is also on EasyPrivacy's block list).

Fixes web gui breaking with ad blockers (e.g. uBlock Origin)
@whisperity
Copy link
Contributor

The problem with this is that this is just a band-aid fix. If the pattern catches on and blockers add the new filename to the blocklists, it will break again. And we can continue eternally renaming.

Wouldn't another approach that defines gtag to be null or some empty shim object that exposes the right functions but does nothing, be a more stable fix here, instead?

@andocz
Copy link
Contributor Author

andocz commented Feb 25, 2022

That's true, I just wanted to keep the solution simple. A more stable fix would be to define gtag inside a <script> element in the html, similarly to how dojoConfig is currently defined.

<script type="text/javascript">
if (Cookies.get('dojo-cache') === undefined) {
Cookies.set('dojo-cache', Date.now(), { expires: 7 });
}
var dojoConfig = {
baseUrl: 'scripts/release/',
tlmSiblingOfDojo: false,
defaultDuration: 1,
async: true,
cacheBust: Cookies.get('dojo-cache'),
packages: [
{ name: 'dojo', location: 'dojo' },
{ name: 'dijit', location: 'dijit' },
{ name: 'dojox', location: 'dojox' },
{ name: 'codecompass', location: 'codecompass' }
]
};
</script>
<script type="text/javascript" src="scripts/release/dojo/dojo.js"></script>

The only downside is the shim definition would have to be copypasted before every inclusion of ga.js (fortunately this only means 2 places currently).

But this approach should be very resilient to ad blockers. Technically uBlock is capable of blocking inline scripts containing a given string (with the script:has-text operator), but filter authors seem to use this type of blocking cautiously: in the default filter list, this operator is only used in domain-specific rules, probably due to its high potential for breaking pages.

Better fix for the ad blocker incompatibility
@andocz
Copy link
Contributor Author

andocz commented Feb 26, 2022

Realized there's a cleaner solution. If window.gtag is used instead of gtag, the ReferenceError is avoided. I changed all occurences of gtag to window.gtag for consistency.

@mcserep
Copy link
Collaborator

mcserep commented Mar 7, 2022

I use AdBlock, which relies on EasyList, which you have also referred to, @andocz. ga.js gets properly loaded for me, I could not reproduce the error related to this PR.

@andocz
Copy link
Contributor Author

andocz commented Mar 7, 2022

I referred to EasyPrivacy, not EasyList. AdBlock Plus doesn't use that one by default, but has an option to use it. Not sure about regular AdBlock.

Edit: Just checked and AdBlock has the option too.

@mcserep
Copy link
Collaborator

mcserep commented Mar 7, 2022

I referred to EasyPrivacy, not EasyList. AdBlock Plus doesn't use that one by default, but has an option to use it. Not sure about regular AdBlock.

Yeah, the URL made the confusion ( easylist.to/easylist/ easyprivacy.txt). I managed to reproduce the issue by adding EasyPrivacy to AdBlock.

@mcserep mcserep merged commit 4c9f38a into Ericsson:master Mar 7, 2022
@mcserep mcserep added this to the Release Gershwin milestone Mar 7, 2022
@mcserep mcserep added Kind: Bug ⚠️ Target: WebGUI Issues related to the web frontend. labels Mar 7, 2022
@andocz andocz deleted the fix-adblock branch March 7, 2022 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kind: Bug ⚠️ Target: WebGUI Issues related to the web frontend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants