@diosmosis opened this Pull Request on April 2nd 2019 Member

Currently when normal non-admin views are rendered in admin controllers, it is assumed they are admin views. Settings for admin views are thus set, which can cause problems for normal views.

For example, say a matomo allows embedding pages, but not settings pages, then a user loads a site with no data. The siteWithoutData() action is invoked, and it is assumed to be an admin controller action, so setBasicVariablesAdminView() is called. This method assumes the view is a settings page so it will disable embedding pages since enable_framed_settings is not enabled.

This PR fixes this by allowing the view type to be specified to renderTemplate() and other methods. It's a bit odd looking, but it's hard to find a way to fix this properly given how many variables get set.

@tsteur commented on April 10th 2019 Member

@diosmosis it seems this is actually breaking API: https://travis-ci.org/matomo-org/matomo/jobs/515032508#L925

The method renderTemplate is marked as <a class='mention' href='https://github.com/api'>@api</a> preventing us from adding any variables because it would be impossible for a plugin to be compatible with previous and newer versions (older versions would not want to see the parameter while newer ones should have). Assuming this isn't used much in the wild we could possibly be fine with the breaking change but would need to adjust tag manager and increase the required Matomo version number for tag manager.

I will look through the detailed code changes later again.

This Pull Request was closed on May 17th 2019
Powered by GitHub Issue Mirror