Issue #2235901 by alexpott, mdrummond, iMiksu, sun, Wim Leers: Remove custom...

Issue #2235901 by alexpott, mdrummond, iMiksu, sun, Wim Leers: Remove custom theme settings from *.info.yml

core/config/schema/core.data_types.schema.yml

@@ -177,6 +177,11 @@ theme_settings:
         use_default:
           type: boolean
           label: 'Use default'
+    third_party_settings:
+      type: sequence
+      label: 'Third party settings'
+      sequence:
+        - type: theme_settings.third_party.[%key]
 
 theme_breakpoints_default:
   type: sequence

core/includes/theme.inc

@@ -323,9 +323,6 @@ function drupal_find_theme_templates($cache, $extension, $path) {
  *
  * The final setting is obtained from the last value found in the following
  * sources:
- * - the default theme-specific settings defined in any base theme's .info.yml
- *   file
- * - the default theme-specific settings defined in the theme's .info.yml file
  * - the saved values from the global theme settings form
  * - the saved values from the theme's settings form
  * To only retrieve the default global theme setting, an empty string should be
@@ -340,6 +337,7 @@ function drupal_find_theme_templates($cache, $extension, $path) {
  *   The value of the requested setting, NULL if the setting does not exist.
  */
 function theme_get_setting($setting_name, $theme = NULL) {
+  /** @var \Drupal\Core\Theme\ThemeSettings[] $cache */
   $cache = &drupal_static(__FUNCTION__, array());
 
   // If no key is given, use the current theme if we can determine it.
@@ -350,33 +348,13 @@ function theme_get_setting($setting_name, $theme = NULL) {
   if (empty($cache[$theme])) {
     // Create a theme settings object.
     $cache[$theme] = new ThemeSettings($theme);
+    // Get the global settings from configuration.
+    $cache[$theme]->setData(\Drupal::config('system.theme.global')->get());
 
-    // Get the values for the theme-specific settings from the .info.yml files
-    // of the theme and all its base themes.
-    $themes = list_themes();
-    if ($theme && isset($themes[$theme])) {
+    $themes = \Drupal::service('theme_handler')->listInfo();
+    if (isset($themes[$theme])) {
       $theme_object = $themes[$theme];
 
-      // Create a list which includes the current theme and all its base themes.
-      if (isset($theme_object->base_themes)) {
-        $theme_keys = array_keys($theme_object->base_themes);
-        $theme_keys[] = $theme;
-      }
-      else {
-        $theme_keys = array($theme);
-      }
-      // Read hard-coded default settings from the theme info files.
-      foreach ($theme_keys as $theme_key) {
-        if (!empty($themes[$theme_key]->info['settings'])) {
-          $cache[$theme]->merge($themes[$theme_key]->info['settings']);
-        }
-      }
-    }
-
-    // Get the global settings from configuration.
-    $cache[$theme]->merge(\Drupal::config('system.theme.global')->get());
-
-    if ($theme && isset($themes[$theme])) {
       // Retrieve configured theme-specific settings, if any.
       try {
         if ($theme_settings = \Drupal::config($theme . '.settings')->get()) {

core/lib/Drupal/Core/Config/ConfigInstaller.php

@@ -90,10 +90,9 @@ public function __construct(ConfigFactoryInterface $config_factory, StorageInter
    */
   public function installDefaultConfig($type, $name) {
     $extension_path = drupal_get_path($type, $name);
-    // If the extension provides configuration schema clear the definitions.
-    if (is_dir($extension_path . '/' . InstallStorage::CONFIG_SCHEMA_DIRECTORY)) {
-      // Refresh the schema cache if installing default configuration and the
-      // extension has a configuration schema directory.
+    // Refresh the schema cache if the extension provides configuration schema
+    // or is a theme.
+    if (is_dir($extension_path . '/' . InstallStorage::CONFIG_SCHEMA_DIRECTORY) || $type == 'theme') {
       $this->typedConfig->clearCachedDefinitions();
     }
 

core/modules/config/src/Tests/ConfigImportUITest.php

@@ -84,7 +84,6 @@ function testImport() {
     $system_theme = \Drupal::config('system.theme')->get();
     $system_theme['default'] = 'bartik';
     $staging->write('system.theme', $system_theme);
-    $staging->write('bartik.settings', $install_storage->read('bartik.settings'));
 
     // Read the action config from module default config folder.
     $action_settings = $install_storage->read('action.settings');
@@ -108,7 +107,6 @@ function testImport() {
     $this->assertText('core.extension');
     $this->assertText('system.theme');
     $this->assertText('action.settings');
-    $this->assertText('bartik.settings');
     $this->assertFieldById('edit-submit', t('Import all'));
 
     // Import and verify that both do not appear anymore.
@@ -118,7 +116,6 @@ function testImport() {
     $this->assertNoText('core.extension');
     $this->assertNoText('system.theme');
     $this->assertNoText('action.settings');
-    $this->assertNoText('bartik.settings');
 
     $this->assertNoFieldById('edit-submit', t('Import all'));
 

core/modules/config/src/Tests/DefaultConfigTest.php

@@ -10,7 +10,9 @@
 use Drupal\config_test\TestInstallStorage;
 use Drupal\Core\Config\InstallStorage;
 use Drupal\Core\Config\TypedConfigManager;
+use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\simpletest\KernelTestBase;
+use Symfony\Component\DependencyInjection\Reference;
 
 /**
  * Tests that default configuration provided by all modules matches schema.
@@ -24,23 +26,46 @@ class DefaultConfigTest extends KernelTestBase {
   /**
    * Modules to enable.
    *
+   * Enable the system module so that system_config_schema_info_alter() fires.
+   *
+   * @var array
+   */
+  public static $modules = array('system', 'config_test');
+
+  /**
+   * Themes which provide default configuration and need enabling.
+   *
+   * If a theme provides default configuration but does not have a schema
+   * because it can rely on schemas added by system_config_schema_info_alter()
+   * then this test needs to enable it.
+   *
    * @var array
    */
-  public static $modules = array('config_test');
+  protected $themes = ['seven'];
+
+  protected function setUp() {
+    parent::setUp();
+    \Drupal::service('theme_handler')->install($this->themes);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function containerBuild(ContainerBuilder $container) {
+    parent::containerBuild($container);
+    $container->register('DefaultConfigTest.schema_storage')
+      ->setClass('\Drupal\config_test\TestInstallStorage')
+      ->addArgument(InstallStorage::CONFIG_SCHEMA_DIRECTORY);
+
+    $definition = $container->getDefinition('config.typed');
+    $definition->replaceArgument(1, new Reference('DefaultConfigTest.schema_storage'));
+  }
 
   /**
    * Tests default configuration data type.
    */
   public function testDefaultConfig() {
-    // Create a typed config manager with access to configuration schema in
-    // every module, profile and theme.
-    $typed_config = new TypedConfigManager(
-      \Drupal::service('config.storage'),
-      new TestInstallStorage(InstallStorage::CONFIG_SCHEMA_DIRECTORY),
-      \Drupal::service('cache.discovery'),
-      \Drupal::service('module_handler')
-    );
-
+    $typed_config = \Drupal::service('config.typed');
     // Create a configuration storage with access to default configuration in
     // every module, profile and theme.
     $default_config_storage = new TestInstallStorage();

core/modules/shortcut/config/schema/shortcut.schema.yml

@@ -10,3 +10,12 @@ shortcut.set.*:
     label:
       type: label
       label: 'Label'
+
+# Schema for theme settings.
+theme_settings.third_party.shortcut:
+  type: mapping
+  label: 'Shortcut settings'
+  mapping:
+    module_link:
+      type: boolean
+      label: 'Add shortcut link'

core/modules/shortcut/shortcut.module

@@ -330,7 +330,7 @@ function shortcut_preprocess_page(&$variables) {
       $route_parameters = array('shortcut' => $shortcut_id);
     }
 
-    if (theme_get_setting('shortcut_module_link')) {
+    if (theme_get_setting('third_party_settings.shortcut.module_link')) {
       $variables['title_suffix']['add_or_remove_shortcut'] = array(
         '#attached' => array(
           'library' => array(

core/modules/system/src/Tests/Theme/ThemeTest.php

@@ -218,17 +218,6 @@ function testListThemes() {
     $this->assertIdentical($themes['test_subtheme']->prefix, 'twig', 'Subtheme\'s object includes the theme engine prefix.');
   }
 
-  /**
-   * Test the theme_get_setting() function.
-   */
-  function testThemeGetSetting() {
-    $this->container->get('theme_handler')->install(array('test_subtheme'));
-    \Drupal::theme()->setActiveTheme(\Drupal::service('theme.initialization')->initTheme('test_theme'));
-    $this->assertIdentical(theme_get_setting('theme_test_setting'), 'default value', 'theme_get_setting() uses the default theme automatically.');
-    $this->assertNotEqual(theme_get_setting('subtheme_override', 'test_basetheme'), theme_get_setting('subtheme_override', 'test_subtheme'), 'Base theme\'s default settings values can be overridden by subtheme.');
-    $this->assertIdentical(theme_get_setting('basetheme_only', 'test_subtheme'), 'base theme value', 'Base theme\'s default settings values are inherited by subtheme.');
-  }
-
   /**
    * Tests child element rendering for 'render element' theme hooks.
    */

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

@@ -11,6 +11,3 @@ stylesheets-override:
 stylesheets-remove:
   - base-remove.css
   - base-remove.sub-override.css
-settings:
-  basetheme_only: 'base theme value'
-  subtheme_override: 'base theme value'

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

@@ -14,5 +14,3 @@ stylesheets-remove:
   - sub-remove.css
   - base-add.sub-remove.css
   - base-override.sub-remove.css
-settings:
-  subtheme_override: 'subtheme value'

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

@@ -16,8 +16,6 @@ base theme: classy
 core: 8.x
 stylesheets-remove:
   - system.module.css
-settings:
-  theme_test_setting: 'default value'
 regions:
   content: Content
   left: Left

core/themes/bartik/bartik.info.yml

@@ -29,6 +29,3 @@ regions:
   footer_thirdcolumn: 'Footer third column'
   footer_fourthcolumn: 'Footer fourth column'
   footer: Footer
-
-settings:
-  shortcut_module_link: false

core/themes/bartik/config/install/bartik.settings.yml

-# @todo There is no UI yet for configuring this, but the setting is included
-#   here, because ConfigImportUITest requires a non-empty bartik.settings.yml
-#   file: https://drupal.org/node/2235901.
-shortcut_module_link: false

core/themes/bartik/config/schema/bartik.schema.yml

@@ -3,9 +3,3 @@
 bartik.settings:
   type: theme_settings
   label: 'Bartik settings'
-  mapping:
-    # @todo Module-specific settings should be defined by the module:
-    #   https://drupal.org/node/2235901.
-    shortcut_module_link:
-      type: boolean
-      label: 'Shortcut module link'

core/themes/seven/config/install/seven.settings.yml

+third_party_settings:
+  shortcut:
+    module_link: true

core/themes/seven/config/schema/seven.schema.yml

@@ -3,7 +3,3 @@
 seven.settings:
   type: theme_settings
   label: 'Seven settings'
-
-seven.breakpoints:
-  type: theme_breakpoints_default
-  label: 'Seven breakpoints'

core/themes/seven/seven.info.yml

@@ -25,6 +25,3 @@ regions:
   sidebar_first: 'First sidebar'
 regions_hidden:
   - sidebar_first
-
-settings:
-  shortcut_module_link: true