Support for noscript tag

Hey @erich93063,

There is currently no support for embedding no script tags using JS injector. How would you imagine this working? Would there be a seperate rule for no script tags? Or would they complement an existing rule perhaps?

In any case, open to patches.

Hi. I would think there would be a different field under the js field. And whereas the js field is enclosed in <script> tags, the other field would be placed under the js on the page and would be enclosed in <noscript> tags. If I were better at PHP I would offer a patch.

Hi

I keep having to add this to a lot of sites so I thought I give it a shot.

This patch adds support for a noscript tag with a region selector (exportable with features).

Hey Michael,

Thanks so much for taking the time to create a patch, all in all this is a nice piece of work, even love the fact that you have written the upgrade path.

Having the <noscript> tags in a region was a novel idea, not too sure what other people think of this, there are a few issues, e.g. weight of the code with respect to other blocks that have not been factored in. I was wondering about just creating a block, and letting the end user place it (thus removing all the block placement code), but this will fragment the visibility rules, so I am conflicted.

I did spot this on simplytest.me:

Warning: Invalid argument supplied for foreach() in js_injector_page_build() (line 177 of /home/s2d62383004f42d4/www/sites/default/modules/js_injector/js_injector.module).

My tiny code review:

1. +++ b/js_injector.module
   @@ -160,10 +160,44 @@ function js_injector_init() {
   +  // taken from admin_menu
   +  // Performance: Skip this entirely for AJAX requests.
   +  if (strpos($_GET['q'], 'js/') === 0) {
   +    return;
   +  }
   

   Is this the best way to do this?

2. +++ b/js_injector.module
   @@ -160,10 +160,44 @@ function js_injector_init() {
   +function _js_inject_active_rules($rule = NULL) {
   

   Missing docblock above function

3. +++ b/plugins/export_ui/js_injector_ctools_export_ui.inc
   @@ -90,6 +90,68 @@ function js_injector_ctools_export_ui_form(&$form, &$form_state) {
   +  // copied from block config, only difference is that we list all regions, not just visible
   

   Watch the 80 character wrap limit, also use sentence case

Hi Sean

thanks for the review. I'm attaching a new patch.

Re 1: I've removed that bit. I had seen it in admin_menu and thought it may make sense but I'll leave it out if it makes no sense.

Re 2: Added

Re 3: I've reformatted some of the text. Most of it is copied from the block module and from the for code for JavaScript in the function. There is a lot of code over the 80 Did you mean this line doesn't have sentence case:

Specify in which themes and regions the noscript tag is injected. In most cases using the Page bottom (default) is recommended. In case this does not work with your theme you can select a different region

For me the main reason to use js_inject is that it supports features. It gets much more complicated when using blocks. I am not too bothered about the weight issue. It doesn't matter really when the noscript is triggered.

Thanks @miiimooo, I have committed this (with slight spacing tweaks) to the 7.x-2.x branch. There will be a new dev release shortly. I have given you git attribution as well. Thanks for the patch.