Issue #575298 by sun, tim.plunkett, Rob Loach, KrisBulman, Jacine,...

Issue #575298 by sun, tim.plunkett, Rob Loach, KrisBulman, Jacine, danillonunes: Provide non-PHP way to reliably override CSS.

core/includes/common.inc

@@ -2564,9 +2564,20 @@ function drupal_add_css($data = NULL, $options = NULL) {
         // key as $data could be a very long string of CSS.
         $css[] = $options;
         break;
+
+      case 'file':
+        // Local CSS files are keyed by basename; if a file with the same
+        // basename is added more than once, it gets overridden.
+        // By default, take over the filename as basename.
+        if (!isset($options['basename'])) {
+          $options['basename'] = drupal_basename($data);
+        }
+        $css[$options['basename']] = $options;
+        break;
+
       default:
-        // Local and external files must keep their name as the associative key
-        // so the same CSS file is not be added twice.
+        // External files are keyed by their full URI, so the same CSS file is
+        // not added twice.
         $css[$data] = $options;
     }
   }
@@ -2604,6 +2615,8 @@ function drupal_add_css($data = NULL, $options = NULL) {
  * @see drupal_add_css()
  */
 function drupal_get_css($css = NULL, $skip_alter = FALSE) {
+  global $theme_info;
+
   if (!isset($css)) {
     $css = drupal_add_css();
   }
@@ -2616,17 +2629,20 @@ function drupal_get_css($css = NULL, $skip_alter = FALSE) {
   // Sort CSS items, so that they appear in the correct order.
   uasort($css, 'drupal_sort_css_js');
 
-  // Remove the overridden CSS files. Later CSS files override former ones.
-  $previous_item = array();
-  foreach ($css as $key => $item) {
-    if ($item['type'] == 'file') {
-      // If defined, force a unique basename for this file.
-      $basename = isset($item['basename']) ? $item['basename'] : drupal_basename($item['data']);
-      if (isset($previous_item[$basename])) {
-        // Remove the previous item that shared the same base name.
-        unset($css[$previous_item[$basename]]);
+  // Allow themes to remove CSS files by basename.
+  if (!empty($theme_info->stylesheets_remove)) {
+    foreach ($css as $key => $options) {
+      if (isset($options['basename']) && isset($theme_info->stylesheets_remove[$options['basename']])) {
+        unset($css[$key]);
+      }
+    }
+  }
+  // Allow themes to conditionally override CSS files by basename.
+  if (!empty($theme_info->stylesheets_override)) {
+    foreach ($css as $key => $options) {
+      if (isset($options['basename']) && isset($theme_info->stylesheets_override[$options['basename']])) {
+        $css[$key]['data'] = $theme_info->stylesheets_override[$options['basename']];
       }
-      $previous_item[$basename] = $key;
     }
   }
 
@@ -2967,24 +2983,11 @@ function drupal_pre_render_styles($elements) {
         elseif ($group['preprocess']) {
           $import = array();
           foreach ($group['items'] as $item) {
-            // A theme's .info file may have an entry for a file that doesn't
-            // exist as a way of overriding a module or base theme CSS file from
-            // being added to the page. Normally, file_exists() calls that need
-            // to run for every page request should be minimized, but this one
-            // is okay, because it only runs when CSS aggregation is disabled.
-            // On a server under heavy enough load that file_exists() calls need
-            // to be minimized, CSS aggregation should be enabled, in which case
-            // this code is not run. When aggregation is enabled,
-            // drupal_load_stylesheet() checks file_exists(), but only when
-            // building the aggregate file, which is then reused for many page
-            // requests.
-            if (file_exists($item['data'])) {
-              // The dummy query string needs to be added to the URL to control
-              // browser-caching. IE7 does not support a media type on the
-              // @import statement, so we instead specify the media for the
-              // group on the STYLE tag.
-              $import[] = '@import url("' . check_plain(file_create_url($item['data']) . '?' . $query_string) . '");';
-            }
+            // The dummy query string needs to be added to the URL to control
+            // browser-caching. IE7 does not support a media type on the
+            // @import statement, so we instead specify the media for the
+            // group on the STYLE tag.
+            $import[] = '@import url("' . check_plain(file_create_url($item['data']) . '?' . $query_string) . '");';
           }
           // In addition to IE's limit of 31 total CSS inclusion tags, it also
           // has a limit of 31 @import statements per STYLE tag.
@@ -3007,14 +3010,6 @@ function drupal_pre_render_styles($elements) {
         else {
           foreach ($group['items'] as $item) {
             $element = $link_element_defaults;
-            // We do not check file_exists() here, because this code runs for
-            // files whose 'preprocess' is set to FALSE, and therefore, even
-            // when aggregation is enabled, and we want to avoid needlessly
-            // taxing a server that may be under heavy load. The file_exists()
-            // performed above for files whose 'preprocess' is TRUE is done for
-            // the benefit of theme .info files, but code that deals with files
-            // whose 'preprocess' is FALSE is responsible for ensuring the file
-            // exists.
             // The dummy query string needs to be added to the URL to control
             // browser-caching.
             $query_string_separator = (strpos($item['data'], '?') !== FALSE) ? '&' : '?';

core/includes/theme.inc

@@ -149,6 +149,10 @@ function _drupal_theme_initialize($theme, $base_theme = array(), $registry_callb
   // We work it this way so that we can have child themes override parent
   // theme stylesheets easily.
   $final_stylesheets = array();
+  // CSS file basenames to override, pointing to the final, overridden filepath.
+  $theme->stylesheets_override = array();
+  // CSS file basenames to remove.
+  $theme->stylesheets_remove = array();
 
   // Grab stylesheets from base theme
   foreach ($base_theme as $base) {
@@ -159,6 +163,18 @@ function _drupal_theme_initialize($theme, $base_theme = array(), $registry_callb
         }
       }
     }
+    $base_theme_path = dirname($base->filename);
+    if (!empty($base->info['stylesheets-remove'])) {
+      foreach ($base->info['stylesheets-remove'] as $basename) {
+        $theme->stylesheets_remove[$basename] = $base_theme_path . '/' . $basename;
+      }
+    }
+    if (!empty($base->info['stylesheets-override'])) {
+      foreach ($base->info['stylesheets-override'] as $name) {
+        $basename = drupal_basename($name);
+        $theme->stylesheets_override[$basename] = $base_theme_path . '/' . $name;
+      }
+    }
   }
 
   // Add stylesheets used by this theme.
@@ -169,6 +185,25 @@ function _drupal_theme_initialize($theme, $base_theme = array(), $registry_callb
       }
     }
   }
+  if (!empty($theme->info['stylesheets-remove'])) {
+    foreach ($theme->info['stylesheets-remove'] as $basename) {
+      $theme->stylesheets_remove[$basename] = $theme_path . '/' . $basename;
+
+      if (isset($theme->stylesheets_override[$basename])) {
+        unset($theme->stylesheets_override[$basename]);
+      }
+    }
+  }
+  if (!empty($theme->info['stylesheets-override'])) {
+    foreach ($theme->info['stylesheets-override'] as $name) {
+      $basename = drupal_basename($name);
+      $theme->stylesheets_override[$basename] = $theme_path . '/' . $name;
+
+      if (isset($theme->stylesheets_remove[$basename])) {
+        unset($theme->stylesheets_remove[$basename]);
+      }
+    }
+  }
 
   // And now add the stylesheets properly
   foreach ($final_stylesheets as $media => $stylesheets) {

core/modules/color/color.module

@@ -77,7 +77,7 @@ function _color_html_alter(&$vars) {
         if (drupal_basename($old_path) == drupal_basename($color_path)) {
           // Replace the path to the new css file.
           // This keeps the order of the stylesheets intact.
-          $vars['css'][$old_path]['data'] = $color_path;
+          $vars['css'][drupal_basename($old_path)]['data'] = $color_path;
         }
       }
     }

core/modules/system/lib/Drupal/system/Tests/Ajax/FrameworkTest.php

@@ -78,8 +78,11 @@ function testLazyLoad() {
       'css' => drupal_get_path('module', 'system') . '/system.admin.css',
       'js' => drupal_get_path('module', 'system') . '/system.js',
     );
+    // CSS files are stored by basename, see drupal_add_css().
+    $expected_css_basename = drupal_basename($expected['css']);
+
     // @todo D8: Add a drupal_css_defaults() helper function.
-    $expected_css_html = drupal_get_css(array($expected['css'] => array(
+    $expected_css_html = drupal_get_css(array($expected_css_basename => array(
       'type' => 'file',
       'group' => CSS_DEFAULT,
       'weight' => 0,
@@ -135,7 +138,7 @@ function testLazyLoad() {
 
     // Verify the expected CSS file was added, both to Drupal.settings, and as
     // an Ajax command for inclusion into the HTML.
-    $this->assertEqual($new_css, $original_css + array($expected['css'] => 1), format_string('Page state now has the %css file.', array('%css' => $expected['css'])));
+    $this->assertEqual($new_css, $original_css + array($expected_css_basename => 1), format_string('Page state now has the %css file.', array('%css' => $expected['css'])));
     $this->assertCommand($commands, array('data' => $expected_css_html), format_string('Page now has the %css file.', array('%css' => $expected['css'])));
 
     // Verify the expected JS file was added, both to Drupal.settings, and as

core/modules/system/lib/Drupal/system/Tests/Common/CascadingStylesheetsTest.php

@@ -48,7 +48,7 @@ function testDefault() {
   function testAddFile() {
     $path = drupal_get_path('module', 'simpletest') . '/simpletest.css';
     $css = drupal_add_css($path);
-    $this->assertEqual($css[$path]['data'], $path, 'Adding a CSS file caches it properly.');
+    $this->assertEqual($css['simpletest.css']['data'], $path);
   }
 
   /**

core/modules/system/lib/Drupal/system/Tests/Theme/ThemeInfoStylesTest.php

+<?php
+
+/**
+ * @file
+ * Contains Drupal\system\Tests\Theme\ThemeInfoStylesTest.
+ */
+
+namespace Drupal\system\Tests\Theme;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests processing of theme .info stylesheets.
+ */
+class ThemeInfoStylesTest extends WebTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('theme_test');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Theme .info styles',
+      'description' => 'Tests processing of theme .info stylesheets.',
+      'group' => 'Theme',
+    );
+  }
+
+  /**
+   * Tests stylesheets-override and stylesheets-remove.
+   */
+  function testStylesheets() {
+    theme_enable(array('test_basetheme', 'test_subtheme'));
+    variable_set('theme_default', 'test_subtheme');
+
+    $base = drupal_get_path('theme', 'test_basetheme');
+    // Unlike test_basetheme (and the original module CSS), the subtheme decides
+    // to put all of its CSS into a ./css subdirectory. All overrides and
+    // removals are expected to be based on a file's basename and should work
+    // nevertheless.
+    $sub = drupal_get_path('theme', 'test_subtheme') . '/css';
+
+    $this->drupalGet('theme-test/info/stylesheets');
+
+    $this->assertRaw("$base/base-add.css");
+    $this->assertRaw("$base/base-override.css");
+    $this->assertNoRaw("base-remove.css");
+
+    $this->assertRaw("$sub/sub-add.css");
+
+    $this->assertRaw("$sub/sub-override.css");
+    $this->assertRaw("$sub/base-add.sub-override.css");
+    $this->assertRaw("$sub/base-remove.sub-override.css");
+
+    $this->assertNoRaw("sub-remove.css");
+    $this->assertNoRaw("base-add.sub-remove.css");
+    $this->assertNoRaw("base-override.sub-remove.css");
+  }
+}

core/modules/system/tests/modules/theme_test/theme_test.module

@@ -59,6 +59,11 @@ function theme_test_menu() {
     'access callback' => TRUE,
     'type' => MENU_CALLBACK,
   );
+  $items['theme-test/info/stylesheets'] = array(
+    'page callback' => 'theme_test_info_stylesheets',
+    'access callback' => TRUE,
+    'type' => MENU_CALLBACK,
+  );
   return $items;
 }
 
@@ -106,6 +111,25 @@ function theme_test_template_test_page_callback() {
   return theme('theme_test_template_test');
 }
 
+/**
+ * Page callback; Adds stylesheets to test theme .info property processing.
+ *
+ * @see test_basetheme.info
+ * @see test_subtheme.info
+ * @see \Drupal\system\Tests\Theme\ThemeInfoStylesTest
+ * @see http://drupal.org/node/967266#comment-3689670
+ */
+function theme_test_info_stylesheets() {
+  $path = drupal_get_path('module', 'theme_test');
+  drupal_add_css("$path/css/base-override.css");
+  drupal_add_css("$path/css/base-override.sub-remove.css");
+  drupal_add_css("$path/css/base-remove.css");
+  drupal_add_css("$path/css/base-remove.sub-override.css");
+  drupal_add_css("$path/css/sub-override.css");
+  drupal_add_css("$path/css/sub-remove.css");
+  return '';
+}
+
 /**
  * Custom theme callback.
  */

core/modules/system/tests/themes/test_basetheme/test_basetheme.info

@@ -3,5 +3,16 @@ description = Test theme which acts as a base theme for other test subthemes.
 core = 8.x
 hidden = TRUE
 
+; Unlike test_subtheme, the base theme does not use a ./css subdirectory.
+stylesheets[all][] = base-add.css
+stylesheets[all][] = base-add.sub-override.css
+stylesheets[all][] = base-add.sub-remove.css
+
+stylesheets-override[] = base-override.css
+stylesheets-override[] = base-override.sub-remove.css
+
+stylesheets-remove[] = base-remove.css
+stylesheets-remove[] = base-remove.sub-override.css
+
 settings[basetheme_only] = base theme value
 settings[subtheme_override] = base theme value

core/modules/system/tests/themes/test_subtheme/test_subtheme.info

@@ -4,4 +4,17 @@ core = 8.x
 base theme = test_basetheme
 hidden = TRUE
 
+; Unlike test_basetheme (and the original module CSS), this subtheme decides to
+; put all of its CSS into a ./css subdirectory. All overrides and removals are
+; expected to be based on a file's basename and should work nevertheless.
+stylesheets[all][] = css/sub-add.css
+
+stylesheets-override[] = css/sub-override.css
+stylesheets-override[] = css/base-add.sub-override.css
+stylesheets-override[] = css/base-remove.sub-override.css
+
+stylesheets-remove[] = sub-remove.css
+stylesheets-remove[] = base-add.sub-remove.css
+stylesheets-remove[] = base-override.sub-remove.css
+
 settings[subtheme_override] = subtheme value

core/modules/system/tests/themes/test_theme/test_theme.info

@@ -3,16 +3,6 @@ description = Theme for testing the theme system
 core = 8.x
 hidden = TRUE
 
-; Normally, themes may list CSS files like this, and if they exist in the theme
-; folder, then they get added to the page. If they have the same file name as a
-; module CSS file, then the theme's version overrides the module's version, so
-; that the module's version is not added to the page. Additionally, a theme may
-; have an entry like this one, without having the corresponding CSS file in the
-; theme's folder, and in this case, it just stops the module's version from
-; being loaded, and does not replace it with an alternate version. We have this
-; here in order for a test to ensure that this correctly prevents the module
-; version from being loaded, and that errors aren't caused by the lack of this
-; file within the theme folder.
-stylesheets[all][] = system.base.css
+stylesheets-remove[] = system.base.css
 
 settings[theme_test_setting] = default value

core/modules/views/lib/Drupal/views/Tests/Plugin/CacheTest.php

@@ -152,7 +152,7 @@ function testHeaderStorage() {
     $js_path = drupal_get_path('module', 'views_test_data') . '/views_cache.test.js';
     $js = drupal_add_js();
 
-    $this->assertTrue(isset($css[$css_path]), 'Make sure the css is added for cached views.');
+    $this->assertTrue(isset($css[basename($css_path)]), 'Make sure the css is added for cached views.');
     $this->assertTrue(isset($js[$js_path]), 'Make sure the js is added for cached views.');
     $this->assertFalse(!empty($view->build_info['pre_render_called']), 'Make sure hook_views_pre_render is not called for the cached view.');
 
@@ -178,7 +178,7 @@ function testHeaderStorage() {
     $css = drupal_add_css();
     $js = drupal_add_js();
 
-    $this->assertFalse(isset($css[$system_css_path]), 'Make sure that unrelated css is not added.');
+    $this->assertFalse(isset($css[basename($system_css_path)]), 'Make sure that unrelated css is not added.');
     $this->assertFalse(isset($js[$system_js_path]), 'Make sure that unrelated js is not added.');
   }
 

core/themes/seven/seven.info

@@ -3,8 +3,14 @@ description = A simple one-column, tableless, fluid width administration theme.
 package = Core
 version = VERSION
 core = 8.x
+
 stylesheets[screen][] = style.css
+stylesheets-override[] = vertical-tabs.css
+stylesheets-override[] = vertical-tabs-rtl.css
+stylesheets-override[] = jquery.ui.theme.css
+
 settings[shortcut_module_link] = 1
+
 regions[content] = Content
 regions[help] = Help
 regions[page_top] = Page top

core/themes/seven/template.php

@@ -105,23 +105,6 @@ function seven_tablesort_indicator($variables) {
   }
 }
 
-/**
- * Implements hook_css_alter().
- */
-function seven_css_alter(&$css) {
-  // Use Seven's vertical tabs style instead of the default one.
-  if (isset($css['core/misc/vertical-tabs.css'])) {
-    $css['core/misc/vertical-tabs.css']['data'] = drupal_get_path('theme', 'seven') . '/vertical-tabs.css';
-  }
-  if (isset($css['core/misc/vertical-tabs-rtl.css'])) {
-    $css['core/misc/vertical-tabs-rtl.css']['data'] = drupal_get_path('theme', 'seven') . '/vertical-tabs-rtl.css';
-  }
-  // Use Seven's jQuery UI theme style instead of the default one.
-  if (isset($css['core/misc/ui/themes/base/jquery.ui.theme.css'])) {
-    $css['core/misc/ui/themes/base/jquery.ui.theme.css']['data'] = drupal_get_path('theme', 'seven') . '/jquery.ui.theme.css';
-  }
-}
-
 /**
  * Implements hook_preprocess_install_page().
  */