Comments

Priority:Normal» Critical

Issue tags:+JavaScript

tag

I've found an issue in Google Analytics and I have no clue how to solve if drupal_add_js() is going to be removed.

Are the designers of this idea able to help here, please?

<?php
/**
* Implements hook_preprocess_search_results().
*
* Collects and adds the number of search results to the head.
*/
function google_analytics_preprocess_search_results(&$variables) {
 
// There is no search result $variable available that hold the number of items
  // found. But the pager item mumber can tell the number of search results.
 
global $pager_total_items;
 
drupal_add_js('window.googleanalytics_search_results = ' . intval($pager_total_items[0]) . ';', array('type' => 'inline', 'group' => JS_LIBRARY-1));
 
// Don't work:
 
$variables['#attached']['js']['window.googleanalytics_search_results = ' . intval($pager_total_items[0]) . ';'] = array('type' => 'inline', 'group' => JS_LIBRARY-1);
}
?>

Adding Contributed project blocker as a removed drupal_add_js() will block a Google Analytics release.

Issue tags:+Prague Hard Problems

  $variables['#attached']['js']['window.googleanalytics_search_results = ' . intval($pager_total_items[0]) . ';'] = array('type' => 'inline', 'group' => JS_LIBRARY-1);

should be

$variables['#attached']['js'][] = array(
  'data' => 'window.googleanalytics_search_results = ' . intval($pager_total_items[0]) . ';',
  'type' => 'inline',
  'group' => JS_LIBRARY-1,
);

And that should work. We have to keep a way to do this because that's how drupalSettings is added to the page. So I wouldn't worry about this use-case.

and if it doesn't, then it means $variables isn't a render array, and #attached is used on a render array so it might be the wrong hook to put that in.

That is the same like your array...

and if it doesn't, then it means $variables isn't a render array, and #attached is used on a render array so it might be the wrong hook to put that in.

This is the only workaround-hook where I'm able to grab the required information I need as the required hooks are missing in core. If you like it or not... this will not work as this $variables is not a render array as I know. It looks like core need to add the appropiate search hooks or drupal_add_js() cannot removed.

Adding more search hooks is fine, please open an issue for that.

As a workaround in preprocess you can set up an #attached array with the js/css/library and call drupal_render() on it. That doesn't fix the root problem (still relies on global state) but gives us at least a central place to fix things. instead of five.

As nod_ said in #7, you're using #attached incorrectly in #4, so this is not true:

That is the same like your array...

@nod_: thx. Gave you the credit for 3 other changes at http://drupalcode.org/project/google_analytics.git/commit/1701fa1

@catch: Cannot get it working...

<?php
  drupal_render
($variables['#attached']['js'][] = array(
   
'data' => 'window.googleanalytics_search_results = ' . intval($pager_total_items[0]) . ';',
   
'type' => 'inline',
   
'group' => JS_LIBRARY-1,
  ));
?>

gives me:

Strict warning: Only variables should be passed by reference in google_analytics_preprocess_search_results() (line 421 of modules\google_analytics\google_analytics.module).
User error: "data" is an invalid render array key in element_children() (line 4535 of core\includes\common.inc).
User error: "type" is an invalid render array key in element_children() (line 4535 of core\includes\common.inc).
User error: "group" is an invalid render array key in element_children() (line 4535 of core\includes\common.inc).

That's a normal PHP error. Set up the variable first, then pass it to drupal_render(). Also you don't need to use $variables here can just be its own thing.

Thanks :þ

Attach first, then do a drupal_render($variables). drupal_render will look for an #attached key.

(edit) yeah like catch says.

Finally done. http://drupalcode.org/project/google_analytics.git/commit/6798d24 thank you all.

Opened two cases to get rid of this wonky code at all:

Maybe you like to join :-)

Issue summary:View changes

Updated issue summary.

Issue summary:View changes
Issue tags:+beta blocker

Issue summary:View changes

Issue summary:View changes

Issue summary:View changes

Issue summary:View changes

Issue summary:View changes

Issue summary:View changes

The work that needed to be done in #2160577: Remove drupal_add_js() + drupal_add_library() from batch.inc was done in another issue, so that's already taken care of.

I reviewed all other issues; most have been RTBC'd.

Much thanks to vijaycs85 for tackling most of them!

Issue summary:View changes

Issue summary:View changes

#2160581: Remove drupal_add_js() + drupal_add_library() from install.core.inc was also committed just now. That leaves just one: #2160593: Remove drupal_add_js() from menu.module/remove inline JavaScript from menu.module Assigned to: Wim Leers, which is blocked on approval by nod_. Almost there!

Title:[META] Remove drupal_add_js()[META] Remove drupal_add_js() calls

I just committed the last of these. :) WOOHOO!!

Now we need to do something to signal to people it's for internal use only. Prefix with an underscore, mark @deprecated?

We're in the same position with drupal_add_css(). There's a bit of discussion on what to do on https://drupal.org/comment/8311981#comment-8311981 downwards.

Might want to open a new issue to do whatever we end up doing to both of these functions, and close these metas as fixed?

#27: that sounds good :)

Issue summary:View changes

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.