diff --git a/skinr.module b/skinr.module index 8b6a3e3..5110eae 100644 --- a/skinr.module +++ b/skinr.module @@ -62,28 +62,6 @@ function skinr_cache_reset() { } /** - * Implements hook_module_implements_alter(). - */ -function skinr_module_implements_alter(&$implementations, $hook) { - // Run Skinr first to avoid issues with other modules during hook_init(). - if ($hook == 'init') { - $skinr['skinr'] = $implementations['skinr']; - unset($implementations['skinr']); - $implementations = array_merge($skinr, $implementations); - } -} - -/** - * Implements hook_init(). - * - * @todo Kill me. Entirely. - */ -function skinr_init() { - module_load_include('inc', 'skinr', 'skinr.handlers'); - skinr_load_includes(); -} - -/** * Implements hook_preprocess(). * * @todo Optimize this function by removing dependencies on @@ -444,6 +422,10 @@ function skinr_implements() { if (!isset($cache)) { $cache = array(); + + // Load built-in support code for Drupal core modules. + module_load_include('inc', 'skinr', 'skinr.handlers'); + // Collect hook_skinr_api_VERSION() module implementations. This will also // auto-load $module.skinr.inc files, which may contain skin/group hook // implementations (when not using the plugin system). @@ -465,9 +447,14 @@ function skinr_implements() { // $module.skinr.inc file and auto-load it. module_implements() only // auto-loads $module.skinr.inc in a module's root folder. if (isset($cache[$module]['path'])) { - $file = DRUPAL_ROOT . '/' . $cache[$module]['path'] . '/' . $module . '.skinr.inc'; - if (file_exists($file)) { - require_once $file; + $file = $cache[$module]['path'] . '/' . $module . '.skinr.inc'; + if (file_exists(DRUPAL_ROOT . '/' . $file)) { + $cache[$module]['include file'] = $file; + // When rebuilding the cache, we auto-load the include file for + // convenience. Calling code should invoke skinr_load_includes() + // on the extensions they want to act with. + // @todo Remove this. + include_once DRUPAL_ROOT . '/' . $file; } } // Populate defaults. @@ -537,9 +524,14 @@ function skinr_implements() { ); // Lastly, for API consistency with modules, check whether the theme // contains a $theme.skinr.inc file and auto-load it, if any. - $file = DRUPAL_ROOT . '/' . $cache[$name]['path'] . '/' . $name . '.skinr.inc'; - if (file_exists($file)) { - require_once $file; + $file = $cache[$name]['path'] . '/' . $name . '.skinr.inc'; + if (file_exists(DRUPAL_ROOT . '/' . $file)) { + $cache[$name]['include file'] = $file; + // When rebuilding the cache, we auto-load the include file for + // convenience. Calling code should invoke skinr_load_includes() + // on the extensions they want to act with. + // @todo Remove this. + include_once DRUPAL_ROOT . '/' . $file; } } } @@ -549,15 +541,15 @@ function skinr_implements() { } /** - * Includes $extension.skinr.inc files of extensions compatible with this version of Skinr. + * Loads $extension.skinr.inc include files of passed in extensions. * - * @todo Shoot me. Twice. + * @todo These hook implementations should not be required to be loaded at all, + * rewrite the code that is currently not using the cache. */ -function skinr_load_includes() { - foreach (skinr_implements() as $extension) { - $file = DRUPAL_ROOT . '/' . $extension['path'] . '/' . $extension['name'] . '.skinr.inc'; - if (file_exists($file)) { - require_once $file; +function skinr_load_includes(array $extensions) { + foreach ($extensions as $extension) { + if (isset($extension['include file'])) { + include_once DRUPAL_ROOT . '/' . $extension['include file']; } } } @@ -1206,6 +1198,11 @@ function skinr_get_config_info() { if (!isset($config_info)) { if ($cached = cache_get('skinr_config_info')) { $config_info = $cached->data; + + // Load the module integration files. + // @todo Whatever currently needs to be invoked has to be cached instead. + skinr_load_includes($config_info); + return $config_info; } $config_info = array(); @@ -1215,6 +1212,13 @@ function skinr_get_config_info() { if (function_exists($function)) { $extension_info = $function(); if (isset($extension_info) && is_array($extension_info)) { + // @todo Yuck! Rewrite hook_skinr_config_info() API structure. Why do + // implementations return info keyed by their own module name? + if (isset($extension['include file'])) { + foreach ($extension_info as &$info) { + $info['include file'] = $extension['include file']; + } + } $config_info = array_merge_recursive($config_info, $extension_info); } } diff --git a/tests/skinr.test b/tests/skinr.test index 2d6ee92..1b78578 100644 --- a/tests/skinr.test +++ b/tests/skinr.test @@ -5,6 +5,46 @@ * Tests for the Skinr module. */ +class SkinrWebTestCase extends DrupalWebTestCase { + /** + * Asserts that a class is set for the given element id. + * + * @param $id + * Id of the HTML element to check. + * @param $class + * The class name to check for. + * @param $message + * Message to display. + * @return + * TRUE on pass, FALSE on fail. + */ + function assertSkinrClass($id, $class, $message = '') { + $elements = $this->xpath('//div[@id=:id and contains(@class, :class)]', array( + ':id' => $id, + ':class' => $class, + )); + $this->assertTrue(!empty($elements[0]), $message); + } + + /** + * Asserts that a class is not set for the given element id. + * + * @param $id + * Id of the HTML element to check. + * @param $class + * The class name to check for. + * @param $message + * Message to display. + * @return + * TRUE on pass, FALSE on fail. + */ + function assertNoSkinrClass($id, $class, $message = '') { + $elements = $this->xpath('//div[@id=:id]', array(':id' => $id)); + $class_attr = (string) $elements[0]['class']; + $this->assertTrue(strpos($class_attr, $class) === FALSE, $message); + } +} + /** * Tests basic module installation. */ @@ -86,7 +126,7 @@ class SkinrInstallationTestCase extends DrupalWebTestCase { * * @link http://drupal.org/node/953336#comment-3738456 Make sure this patch is applied to drupal core @endlink */ -class SkinrApiTestCase extends DrupalWebTestCase { +class SkinrApiTestCase extends SkinrWebTestCase { protected $profile = 'testing'; public static function getInfo() { @@ -140,6 +180,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { 'node' => array( 'version' => VERSION, 'path' => drupal_get_path('module', 'skinr') . '/modules', + 'include file' => drupal_get_path('module', 'skinr') . '/modules/node.skinr.inc', ), // skinr_test has been installed. 'skinr_test' => array( @@ -151,6 +192,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { 'directory' => 'skins', 'base themes' => array(), 'sub themes' => drupal_map_assoc(array('skinr_test_subtheme')), + 'include file' => drupal_get_path('theme', 'skinr_test_basetheme') . '/skinr_test_basetheme.skinr.inc', ), 'skinr_test_subtheme' => array( 'type' => 'theme', @@ -158,6 +200,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { 'directory' => 'skins', 'base themes' => drupal_map_assoc(array('skinr_test_basetheme')), 'sub themes' => array(), + 'include file' => drupal_get_path('theme', 'skinr_test_subtheme') . '/skinr_test_subtheme.skinr.inc', ), ); // When running tests on Skinr code packaged by drupal.org, all 'version' @@ -189,6 +232,61 @@ class SkinrApiTestCase extends DrupalWebTestCase { } /** + * Tests skinr_implements() caching and auto-loading. + */ + function testSkinrImplementsCache() { + module_enable(array('block')); + // Enable main system block for content region and the user menu block for + // the first sidebar. + $default_theme = variable_get('theme_default', 'bartik'); + db_merge('block') + ->key(array( + 'theme' => $default_theme, + 'module' => 'system', + 'delta' => 'main', + )) + ->fields(array( + 'status' => 1, + 'region' => 'content', + 'pages' => '', + )) + ->execute(); + db_merge('block') + ->key(array( + 'theme' => $default_theme, + 'module' => 'system', + 'delta' => 'powered-by', + )) + ->fields(array( + 'status' => 1, + 'region' => 'sidebar_first', + 'pages' => '', + )) + ->execute(); + + // Enable a skin defined in an include file, which applies to a module + // element that is equally registered in an include file (built-in Block + // module integration). + $skin = (object) array( + 'theme' => $default_theme, + 'module' => 'block', + 'element' => 'system__powered-by', + 'skin' => 'skinr_test_font', + 'options' => array('font_1'), + 'status' => 1, + ); + skinr_skin_save($skin); + + // Verify the skin is contained in the output. + $this->drupalGet(''); + $this->assertSkinrClass('block-system-powered-by', 'font-1', 'Skin found.'); + + // Once again, so we hit the cache. + $this->drupalGet(''); + $this->assertSkinrClass('block-system-powered-by', 'font-1', 'Skin found.'); + } + + /** * Tests hook_skinr_skin_info(). */ public function testSkinrSkinInfo() { @@ -422,7 +520,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { * * @link http://drupal.org/node/953336#comment-3738456 Make sure this patch is applied to drupal core @endlink */ -class SkinrDisplayTestCase extends DrupalWebTestCase { +class SkinrDisplayTestCase extends SkinrWebTestCase { protected $profile = 'testing'; public static function getInfo() { @@ -474,42 +572,6 @@ class SkinrDisplayTestCase extends DrupalWebTestCase { theme_enable(array('garland')); } - /** - * Asserts that a class is set for the given element id. - * - * @param $id - * Id of the HTML element to check. - * @param $class - * The class name to check for. - * @param $message - * Message to display. - * @return - * TRUE on pass, FALSE on fail. - */ - function assertSkinrClass($id, $class, $message = '') { - $elements = $this->xpath('//div[@id=:id]', array(':id' => $id)); - $class_attr = (string) $elements[0]['class']; - $this->assertTrue(strpos($class_attr, ' ' . $class . ' '), $message); - } - - /** - * Asserts that a class is not set for the given element id. - * - * @param $id - * Id of the HTML element to check. - * @param $class - * The class name to check for. - * @param $message - * Message to display. - * @return - * TRUE on pass, FALSE on fail. - */ - function assertNoSkinrClass($id, $class, $message = '') { - $elements = $this->xpath('//div[@id=:id]', array(':id' => $id)); - $class_attr = (string) $elements[0]['class']; - $this->assertFalse(strpos($class_attr, ' ' . $class . ' '), $message); - } - public function testSkinrDisplayed() { // Save a skin configuration object. $skin = (object) array(