AOEpeople / Aoe_TemplateHints

Advanced Template Hints for Magento
http://www.fabrizio-branca.de/magento-advanced-template-hints-20.html
GNU General Public License v3.0
189 stars 66 forks source link

Extended activation configuration / depend for PHPStorm URL #21

Open amenk opened 11 years ago

fbrnc commented 11 years ago

Hi,

thanks again for another contribution.

First of all a small optimization: Let's check the store parameter at the end of the if() as reading from the request object is faster.

Now the problem I'm running into: When having the module set to "always on" and to "module enabled" I cannot access the backend anymore. I'm not sure if this is a problem related to my dev environment or if this is a generic solution. Can you confirm?

In addition to that I don't know if we should have another option to have the module enabled or not. I guess it makes sense with the always on feature. But it might be confusion for user that are upgrading or installing it without checking the options. We might enable it by default. The protection should be done through dev mode anyways.

amenk commented 11 years ago

First:

What is the problem accessing the admin interface? I have checked it (actually in a Magento 1.6.2) and it is basically working. But the layout is a little bit different - what kind of problem are you running into?

Second:

Basically there is a general problem in Magento with the dev mode, as the IP filter is blank by default. But at least the template hints in Magento are off by default. Now the problem with your template hints is that they are "invisible" by default - I find this risky because somebody could install them and then forget them while it is still possible to enabled them using ?ath=1 ...

Of course it is generall question if that is the responsibility of the module or if the users should just think about that themselves ... on the other hand security-by-default is not a bad thing.

If we leave module enabled on, I think we should have always on also on, so at least there is something visible after installing the module.

To conclude I do not find it a big regression if the module is disabled by default and you are forced to enabled it in the backend. -> we could write a note in the readme.

Better the users are forced to read & enable it than having it accidentally on.