Skip to content
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

During installer, better detect adblockers that may block Matomo css/js files #12493

Merged
merged 2 commits into from Mar 23, 2018

Conversation

Findus23
Copy link
Member

extends #5094, fixes #11660 and hopefully also fixes #10652

I just remembered that I once opened an issue about AdBlock detection.

Improvements:

  • also add the detection to the installer (as it is most important there)
  • add warning to the top of the page instead of end as nobody scrolls down
  • add an additional check if CSS and JS are loading correctly by hiding a warning
    • I only added it to the installer as it may cause flashes of the warning
    • also catches cases where the JS and CSS generated by the server are broken
    • also warns when user has disabled Javascript
      • should work when one uses NoScript or similar

This needs some testing as there are a lot of edge cases that I didn't test.

I have no idea how the error message should read, so it's just a placeholder

also check for CSS and JS
@mattab
Copy link
Member

mattab commented Jan 22, 2018

It will be useful to have adblocker detection in the web installer!
Feedback:

  • the message "your browser was unable to load the scripts on this page" is flashing at the top of the installer. As it creates a jumping effect, could we move this particular message to the footer of the installer? or is there maybe another solution to avoid the jumping and flashing effects?

@Findus23
Copy link
Member Author

That's what I expected. I honestly don't know a good solution, I tried moving
https://github.com/matomo-org/matomo/pull/12493/files#diff-8796dfce26b44da35489af48780b6c0fR23
out of the document.ready, but at this time the element doesn't exist yet.

I'm open for suggestions on how to improve it.

@fdellwing
Copy link
Contributor

Does anything speaks against setting visibility: hidden instead of display: none?

https://www.thoughtco.com/display-none-vs-visibility-hidden-3466884

@Findus23
Copy link
Member Author

Yes, if I'm not mistaken it doesn't remove the element from the flow which would cause an empty space at the top of every page.

Does anything speak for using it?

@fdellwing
Copy link
Contributor

Well, you asked for ways to not have the jumping effect. That is a way ;)

@mattab
Copy link
Member

mattab commented Mar 20, 2018

I'm open for suggestions on how to improve it.

@Findus23 Maybe we could move the Adblocker message to the bottom of the page so there is no jumping effect, yet the message would still be visible above the fold for most users?

@Findus23
Copy link
Member Author

@mattab On the other hand I opened the pull request mainly because the warning is very far below on the site and isn't really visible (especially when there is no CSS and therefore all languages are shown on a separate line

@mattab
Copy link
Member

mattab commented Mar 23, 2018

Simply moved the notice messages to the bottom to avoid the jumping. It still shows up when the page loads for a few milliseconds, which is not ideal, but maybe better than nothing. @Findus23 maybe you could in a new PR somehow reuse the same technique that is used in core, where the messages shows at the top in case of adblocker, but it does not jump/flash on screen?

@mattab mattab merged commit 668ab5a into 3.x-dev Mar 23, 2018
@mattab mattab deleted the better-adblock-detection branch March 23, 2018 03:46
@mattab mattab changed the title better adblock detection During installer, better detect adblockers that may block Matomo css/js files Mar 23, 2018
InfinityVoid pushed a commit to InfinityVoid/matomo that referenced this pull request Oct 11, 2018
* better adblock detection

also check for CSS and JS

* Move the adblock notice messages to the bottom
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants