Remove JS helper functions [#2454933]

With #2121253: Attach media browser JavaScript settings and #2316707: Drupal.settings.media values become an array of when multiple fields are used the media browser settings were "simplified" and moved to a library which makes the media_attach_browser_js() and media_browser_js() helper functions unnecessary, since they require more code and the loading of a file, compared to simply #attaching the library.

Ex:

module_load_include('inc', 'media', 'includes/media.browser');
$javascript = media_browser_js();

foreach ($javascript as $key => $definitions) {
  foreach ($definitions as $definition) {
    $element['#attached'][$key][] = $definition;
  }
}

vs.

$element['#attached']['library'][] = array('media', 'media_browser');
$element['#attached']['library'][] = array('media', 'media_browser_settings');

The media_browser library should probably depend on the settings library anyway, which would remove the second line as well.

Comment	File	Size	Author
#5	Issue_2454933.patch	1 KB	pol
#1	remove-js-attach-functions-2454933-1.patch	2.81 KB	devin carlson

Comments

Comment #1

devin carlson commented 18 March 2015 at 17:33

Status:

Active

» Needs review

Status	File	Size
new	remove-js-attach-functions-2454933-1.patch	2.81 KB

A patch to remove the functions.

Comment #2

devin carlson commented 18 March 2015 at 19:36

Status:

Needs review

» Fixed

Tested #1 and committed to Media 7.x-2.x.

Comment #3

18 March 2015 at 19:37

Devin Carlson committed b88290c on 7.x-2.x

Issue #2454933 by Devin Carlson: Removed JS helper functions.

Comment #4

steel-track commented 18 March 2015 at 21:53

Just a heads up that this causes a function undefined error with the CKeditor module's plugin loading:

ckeditor/includes/ckeditor.lib.inc line 893

if (array_key_exists('media', $settings['loadPlugins']) && module_exists('media')) {
      module_load_include('inc', 'media', 'includes/media.browser');
      $javascript = media_browser_js();
      foreach ($javascript as $key => $definitions) {
        foreach ($definitions as $definition) {
          $function = 'drupal_add_' . $key;
          call_user_func_array($function, $definition);
        }
      }
      drupal_add_js(drupal_get_path('module', 'ckeditor') . '/plugins/media/library.js', array('scope' => 'footer', 'weight' => -20));
    }
  }

Are they aware of this change?