Unverified Commit 458e132a authored by alexpott's avatar alexpott
Browse files

Issue #3115088 by lauriii, bnjmnm, tim.plunkett: Remove Classy as a base theme of core themes

parent b496f2d6
......@@ -13,5 +13,5 @@ dependencies:
module:
- block_test
theme:
- classy
- stark
visibility: { }
......@@ -276,7 +276,6 @@ public function testBuildContentsCssJSSetting() {
// Enable the Bartik theme, which specifies a CKEditor stylesheet.
\Drupal::service('theme_installer')->install(['bartik']);
$this->config('system.theme')->set('default', 'bartik')->save();
$expected[] = file_url_transform_relative(file_create_url('core/themes/classy/css/components/media-embed-error.css')) . $query_string;
$expected[] = file_url_transform_relative(file_create_url('core/themes/bartik/css/base/elements.css')) . $query_string;
$expected[] = file_url_transform_relative(file_create_url('core/themes/bartik/css/components/captions.css')) . $query_string;
$expected[] = file_url_transform_relative(file_create_url('core/themes/bartik/css/components/table.css')) . $query_string;
......
......@@ -84,8 +84,8 @@ public function testImport() {
$core_extension['module']['automated_cron'] = 0;
$core_extension['module']['ban'] = 0;
$core_extension['module'] = module_config_sort($core_extension['module']);
// Bartik is a subtheme of classy so classy must be enabled.
$core_extension['theme']['classy'] = 0;
// Bartik is a subtheme of Stable so Stable must be enabled.
$core_extension['theme']['stable'] = 0;
$core_extension['theme']['bartik'] = 0;
$sync->write('core.extension', $core_extension);
......@@ -503,10 +503,10 @@ public function testExtensionValidation() {
unset($core['module']['text']);
$module_data = $this->container->get('extension.list.module')->getList();
$this->assertTrue(isset($module_data['node']->requires['text']), 'The Node module depends on the Text module.');
// Bartik depends on classy.
unset($core['theme']['classy']);
// Bartik depends on Stable.
unset($core['theme']['stable']);
$theme_data = \Drupal::service('theme_handler')->rebuildThemeData();
$this->assertTrue(isset($theme_data['bartik']->requires['classy']), 'The Bartik theme depends on the Classy theme.');
$this->assertTrue(isset($theme_data['bartik']->requires['stable']), 'The Bartik theme depends on the Stable theme.');
// This module does not exist.
$core['module']['does_not_exist'] = 0;
// This theme does not exist.
......@@ -516,7 +516,7 @@ public function testExtensionValidation() {
$this->drupalPostForm('admin/config/development/configuration', [], t('Import all'));
$this->assertText('The configuration cannot be imported because it failed validation for the following reasons:');
$this->assertText('Unable to uninstall the Text module since the Node module is installed.');
$this->assertText('Unable to uninstall the Classy theme since the Bartik theme is installed.');
$this->assertText('Unable to uninstall the Stable theme since the Bartik theme is installed.');
$this->assertText('Unable to install the does_not_exist module since it does not exist.');
$this->assertText('Unable to install the does_not_exist theme since it does not exist.');
}
......
......@@ -13,6 +13,7 @@
use Drupal\Core\Entity\Entity\EntityFormDisplay;
use Drupal\Core\Entity\Entity\EntityViewDisplay;
use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Extension\Exception\UnknownExtensionException;
use Drupal\Core\Field\Plugin\Field\FieldWidget\EntityReferenceAutocompleteWidget;
/**
......@@ -316,3 +317,18 @@ function system_post_update_entity_revision_metadata_bc_cleanup() {
$last_installed_schema_repository->setLastInstalledDefinition($entity_type);
}
}
/**
* Uninstall Classy if it is no longer needed.
*/
function system_post_update_uninstall_classy() {
/** @var \Drupal\Core\Extension\ThemeInstallerInterface $theme_installer */
$theme_installer = \Drupal::getContainer()->get('theme_installer');
try {
$theme_installer->uninstall(['classy']);
}
catch (\InvalidArgumentException | UnknownExtensionException $exception) {
// Exception is thrown if Classy wasn't installed or if there are themes
// depending on it.
}
}
<?php
namespace Drupal\Tests\system\Functional\Update;
use Drupal\FunctionalTests\Update\UpdatePathTestBase;
/**
* Ensures that update hook uninstalls Classy when it's no longer needed.
*
* @group Update
* @see system_post_update_uninstall_classy()
*/
class ClassyUninstallUpdateTest extends UpdatePathTestBase {
/**
* {@inheritdoc}
*/
protected function setDatabaseDumpFiles() {
$this->databaseDumpFiles = [
__DIR__ . '/../../../fixtures/update/drupal-8.8.0.bare.standard.php.gz',
];
}
/**
* Ensures that Classy is disabled if it's no longer needed.
*/
public function testUpdate() {
/** @var \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler */
$theme_handler = $this->container->get('theme_handler');
$this->assertTrue($theme_handler->themeExists('classy'));
$this->runUpdates();
// Ensure that Classy is not installed after running updates.
$theme_handler->refreshInfo();
$this->assertFalse($theme_handler->themeExists('classy'));
$this->assertTrue($theme_handler->themeExists('stable'));
}
/**
* Ensures that updates run without errors when Classy is not installed.
*/
public function testUpdateClassyNotInstalled() {
/** @var \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler */
$theme_handler = $this->container->get('theme_handler');
$theme_list = array_keys($theme_handler->listInfo());
/** @var \Drupal\Core\Extension\ThemeInstallerInterface $theme_installer */
$theme_installer = $this->container->get('theme_installer');
$theme_installer->install(['stark']);
$this->container->get('config.factory')
->getEditable('system.theme')
->set('default', 'stark')
->set('admin', '')
->save();
$theme_handler->refreshInfo();
// Uninstall all themes that were installed prior to enabling Stark.
$theme_installer->uninstall($theme_list);
// Ensure that Classy is not installed anymore.
$theme_handler->refreshInfo();
$this->assertFalse($theme_handler->themeExists('classy'));
$this->runUpdates();
$theme_handler->refreshInfo();
$this->assertFalse($theme_handler->themeExists('classy'));
}
/**
* Ensures that updates run without errors when Classy is still needed.
*/
public function testUpdateClassyNeeded() {
/** @var \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler */
$theme_handler = $this->container->get('theme_handler');
/** @var \Drupal\Core\Extension\ThemeInstallerInterface $theme_installer */
$theme_installer = $this->container->get('theme_installer');
$theme_installer->install(['test_theme']);
$this->assertTrue($theme_handler->themeExists('classy'));
$this->runUpdates();
// Ensure that Classy is still installed after running tests.
$theme_handler->refreshInfo();
$this->assertTrue($theme_handler->themeExists('classy'));
}
}
......@@ -409,7 +409,6 @@ public function testUpdatedSite() {
// Make sure our themes are still enabled.
$expected_enabled_themes = [
'bartik',
'classy',
'seven',
'stark',
];
......
/**
* @file
* Image upload widget.
*
* This CSS file is not used in this theme (Classy). It was intended to be used,
* but due to a bug, Drupal 8 shipped with it not being used. To not break
* backwards compatibility, we continue to not load it in Classy. Every
* subtheme of Classy is encouraged to use it, by attaching the
* classy/image-widget asset library in their image-widget.html.twig file.
*
* @see core/themes/seven/templates/content-edit/image-widget.html.twig.
*
* @todo In Drupal 9, let core/themes/classy/templates/content-edit/image-widget.html.twig
* attach the classy/image-widget asset library.
*/
.image-preview {
float: left; /* LTR */
padding: 0 10px 10px 0; /* LTR */
}
[dir="rtl"] .image-preview {
float: right;
padding: 0 0 10px 10px;
}
.image-widget-data {
float: left; /* LTR */
}
[dir="rtl"] .image-widget-data {
float: right;
}
.image-widget-data .text-field {
width: auto;
}
name: Umami
type: theme
base theme: classy
base theme: stable
description: 'The theme used for the Umami food magazine demonstration site.'
version: VERSION
libraries:
- umami/classy.base
- core/normalize
- umami/global
- umami/messages
- umami/webfonts-open-sans
- umami/webfonts-scope-one
libraries-override:
classy/base: umami/classy.base
classy/book-navigation: umami/classy.book-navigation
classy/dialog: umami/classy.dialog
classy/dropbutton: umami/classy.dropbutton
classy/file: umami/classy.file
classy/forum: umami/classy.forum
classy/image-widget: umami/classy.image-widget
classy/indented: umami/classy.indented
classy/media_embed_ckeditor_theme: umami/classy.media_embed_ckeditor_theme
classy/media_embed_error: umami/classy.media_embed_error
classy/media_library: umami/classy.media_library
classy/messages: false
classy/node: umami/classy.node
classy/progress: umami/classy.progress
classy/search-results: umami/classy.search-results
classy/user: umami/classy.user
layout_builder/twocol_section:
css:
theme:
......
......@@ -189,12 +189,6 @@ classy.forum:
component:
css/classy/components/forum.css: { weight: -10 }
classy.image-widget:
version: VERSION
css:
component:
css/classy/components/image-widget.css: {}
classy.indented:
version: VERSION
css:
......
......@@ -597,7 +597,6 @@ public function testUnmetDependency() {
'Unable to install the <em class="placeholder">unknown_module</em> module since it does not exist.',
'Unable to install the <em class="placeholder">Book</em> module since it requires the <em class="placeholder">Node, Text, Field, Filter, User</em> modules.',
'Unable to install the <em class="placeholder">unknown_theme</em> theme since it does not exist.',
'Unable to install the <em class="placeholder">Bartik</em> theme since it requires the <em class="placeholder">Classy</em> theme.',
'Unable to install the <em class="placeholder">Bartik</em> theme since it requires the <em class="placeholder">Stable</em> theme.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.config</em> depends on the <em class="placeholder">unknown</em> configuration that will not exist after import.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.existing</em> depends on the <em class="placeholder">config_test.dynamic.dotted.deleted</em> configuration that will not exist after import.',
......@@ -611,7 +610,6 @@ public function testUnmetDependency() {
'Unable to install the <em class="placeholder">unknown_module</em> module since it does not exist.',
'Unable to install the <em class="placeholder">Book</em> module since it requires the <em class="placeholder">Node, Text, Field, Filter, User</em> modules.',
'Unable to install the <em class="placeholder">unknown_theme</em> theme since it does not exist.',
'Unable to install the <em class="placeholder">Bartik</em> theme since it requires the <em class="placeholder">Classy</em> theme.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.config</em> depends on the <em class="placeholder">unknown</em> configuration that will not exist after import.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.existing</em> depends on the <em class="placeholder">config_test.dynamic.dotted.deleted</em> configuration that will not exist after import.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.module</em> depends on the <em class="placeholder">unknown</em> module that will not be installed after import.',
......@@ -641,7 +639,6 @@ public function testUnmetDependency() {
'Unable to install the <em class="placeholder">unknown_module</em> module since it does not exist.',
'Unable to install the <em class="placeholder">Book</em> module since it requires the <em class="placeholder">Node, Text, Field, Filter, User</em> modules.',
'Unable to install the <em class="placeholder">unknown_theme</em> theme since it does not exist.',
'Unable to install the <em class="placeholder">Bartik</em> theme since it requires the <em class="placeholder">Classy</em> theme.',
'Unable to install the <em class="placeholder">Bartik</em> theme since it requires the <em class="placeholder">Stable</em> theme.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.config</em> depends on the <em class="placeholder">unknown</em> configuration that will not exist after import.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.existing</em> depends on the <em class="placeholder">config_test.dynamic.dotted.deleted</em> configuration that will not exist after import.',
......@@ -651,7 +648,6 @@ public function testUnmetDependency() {
'Unable to install the <em class="placeholder">unknown_module</em> module since it does not exist.',
'Unable to install the <em class="placeholder">Book</em> module since it requires the <em class="placeholder">Node, Text, Field, Filter, User</em> modules.',
'Unable to install the <em class="placeholder">unknown_theme</em> theme since it does not exist.',
'Unable to install the <em class="placeholder">Bartik</em> theme since it requires the <em class="placeholder">Classy</em> theme.',
'Unable to install the <em class="placeholder">Bartik</em> theme since it requires the <em class="placeholder">Stable</em> theme.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.config</em> depends on configuration (<em class="placeholder">unknown, unknown2</em>) that will not exist after import.',
'Configuration <em class="placeholder">config_test.dynamic.dotted.existing</em> depends on the <em class="placeholder">config_test.dynamic.dotted.deleted</em> configuration that will not exist after import.',
......
......@@ -130,7 +130,6 @@ public function providerTestClassyCopies() {
'form.css',
'forum.css',
'icons.css',
'image-widget.css',
'inline-form.css',
'item-list.css',
'link.css',
......@@ -535,7 +534,6 @@ public function providerTestClassyCopies() {
'ui-dialog.css',
'user.css',
'item-list.css',
'image-widget.css',
'field.css',
'tablesort.css',
'tabs.css',
......@@ -550,7 +548,6 @@ public function providerTestClassyCopies() {
'form.css',
'exposed-filters.css',
'tabledrag.css',
'indented.css',
'messages.css',
'pager.css',
'search-results.css',
......
......@@ -27,7 +27,7 @@ public function testMaintenanceTheme() {
$base_themes = $active_theme->getBaseThemeExtensions();
$base_theme_names = array_keys($base_themes);
$this->assertSame(['classy', 'stable'], $base_theme_names);
$this->assertSame(['stable'], $base_theme_names);
}
}
<?php
namespace Drupal\KernelTests\Core\Theme;
use Drupal\KernelTests\KernelTestBase;
/**
* Tests that themes do not depend on Classy libraries.
*
* These tests exist to facilitate the process of decoupling theme from Classy.
* The decoupling process includes replacing the use of all Classy libraries
* with theme-specific ones. These tests ensure these replacements are properly
* implemented.
*
* @group Theme
*/
class ThemeNotUsingClassyLibraryTest extends KernelTestBase {
/**
* The theme initialization.
*
* @var \Drupal\Core\Theme\ThemeInitializationInterface
*/
protected $themeInitialization;
/**
* The library discovery service.
*
* @var \Drupal\Core\Asset\LibraryDiscoveryInterface
*/
protected $libraryDiscovery;
/**
* The theme handler.
*
* @var \Drupal\Core\Extension\ThemeHandlerInterface
*/
protected $themeHandler;
/**
* Classy's libraries.
*
* These are the libraries defined in classy.libraries.yml.
*
* @var string[][]
*
* @see \Drupal\Core\Asset\LibraryDiscoveryInterface::getLibrariesByExtension()
*/
protected $classyLibraries;
/**
* Libraries that Classy extends.
*
* These are the libraries listed in `libraries-extend` in classy.info.yml.
*
* @var string[][]
*/
protected $classyLibrariesExtend;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->themeInitialization = $this->container->get('theme.initialization');
$this->libraryDiscovery = $this->container->get('library.discovery');
$this->themeHandler = $this->container->get('theme_handler');
$this->container->get('theme_installer')->install([
'umami',
'bartik',
'seven',
'claro',
]);
$this->classyLibraries = $this->libraryDiscovery->getLibrariesByExtension('classy');
$this->assertNotEmpty($this->classyLibraries);
$this->classyLibrariesExtend = $this->themeHandler->getTheme('classy')->info['libraries-extend'];
$this->assertNotEmpty($this->classyLibrariesExtend);
}
/**
* Ensures that a theme is decoupled from Classy libraries.
*
* This confirms that none of the libraries defined in classy.libraries.yml
* are loaded by the current theme. For this to happen, the current theme
* must override the Classy library so no assets from Classy are loaded.
*
* @param string $theme
* The theme being tested.
* @param string[] $libraries_to_skip
* Libraries excluded from the test.
*
* @dataProvider providerTestThemeNotUsingClassyLibraries
*/
public function testThemeNotUsingClassyLibraries($theme, array $libraries_to_skip) {
// In some cases an overridden Classy library does not use any copied assets
// from Classy. This array collects those so this test knows to skip
// assertions specific to those copied assets.
$skip_asset_matching_assertions = [];
$theme_path = $this->themeHandler->getTheme($theme)->getPath();
// A list of all libraries that the current theme is overriding. In a
// theme's info.yml file, these are the libraries listed in
// `libraries-override:`, and are libraries altered by the current theme.
// This will be used for confirming that all of Classy's libraries are
// overridden.
$theme_library_overrides = $this->themeInitialization->getActiveThemeByName($theme)->getLibrariesOverride()[$theme_path] ?? [];
// A list of all libraries created by the current theme.
$theme_libraries = $this->libraryDiscovery->getLibrariesByExtension($theme);
$this->assertNotEmpty($theme_libraries);
// Loop through all libraries overridden by the theme. For those that are
// Classy libraries, confirm that the overrides prevent the loading of any
// Classy asset.
foreach ($theme_library_overrides as $library_name => $library_definition) {
$in_skip_list = in_array(str_replace('classy/', '', $library_name), $libraries_to_skip);
// If the library name does not begin with `classy/`, it's not a Classy
// library.
$not_classy_library = substr($library_name, 0, 7) !== 'classy/';
// If $library_definition is false or a string, the override is preventing
// the Classy library from loading altogether.
$library_fully_replaced = $library_definition === FALSE || gettype($library_definition) === 'string';
// If the library is fully replaced, it may need to be added to the
// $skip_asset_matching_assertions array.
if ($library_fully_replaced) {
// Libraries with names that begin with `$theme/classy.` are copies of
// Classy libraries.
$not_copied_from_classy = gettype($library_definition) === 'string' && substr($library_definition, 0, (8 + strlen($theme))) !== "$theme/classy.";
// If the overridden library is not copied from Classy or is FALSE (i.e.
// not loaded at all), it is customized and should skip the tests that
// check for a 1:1 asset match between the Classy library and its
// override in the current theme.
if ($library_definition === FALSE || $not_copied_from_classy) {
$skip_asset_matching_assertions[] = $library_name;
}
}
// If any of these three conditions are true, there's no need for the
// remaining asset-specific assertions in this loop.
if ($in_skip_list || $not_classy_library || $library_fully_replaced) {
continue;
}
// If the library override has a 'css' key, some Classy CSS files may
// still be loading. Confirm this is not the case.
if (isset($library_definition['css'])) {
$this->confirmNoClassyAssets($library_name, $library_definition, 'css');
// If the override has no JS and all Classy CSS is accounted for, add it
// to the list of libraries already fully overridden. It won't be
// necessary to copy the library from Classy.
if (!isset($library_definition['js'])) {
$skip_asset_matching_assertions[] = $library_name;
}
}
if (isset($library_definition['js'])) {
$this->confirmNoClassyAssets($library_name, $library_definition, 'js');
// CSS has already been checked. So, if all JS in the library is
// accounted for, add it to the list of libraries already fully
// overridden. It won't be necessary to copy the library from Classy.
$skip_asset_matching_assertions[] = $library_name;
}
}
// Confirm that every Classy library is copied or fully overridden by the
// current theme.
foreach ($this->classyLibraries as $classy_library_name => $classy_library) {
// If a Classy library is in the $skip_asset_matching_assertions
// array, it does not use any assets copied from Classy and can skip the
// tests in this loop.
$fully_overridden = in_array("classy/$classy_library_name", $skip_asset_matching_assertions);
$skip = in_array($classy_library_name, $libraries_to_skip);
if ($skip || $fully_overridden) {
continue;
}
// Confirm the Classy Library is overridden so assets aren't loaded twice.
$this->assertArrayHasKey("classy/$classy_library_name", $theme_library_overrides, "The classy/$classy_library_name library is not overridden in $theme");
// Confirm there is a theme-specific version of the Classy library.
$this->assertArrayHasKey("classy.$classy_library_name", $theme_libraries, "There is not a $theme equivalent for classy/$classy_library_name");
$theme_copy_of_classy_library = $theme_libraries["classy.$classy_library_name"];
// If the Classy library includes CSS, confirm the theme's copy has the
// same CSS with the same properties.
if (!empty($classy_library['css'])) {
$this->confirmMatchingAssets($classy_library_name, $classy_library, $theme_copy_of_classy_library, $theme_path, 'css');
}
// If the Classy library includes JavaScript, confirm the theme's copy has
// the same JavaScript with the same properties.
if (!empty($classy_library['js'])) {
$this->confirmMatchingAssets($classy_library_name, $classy_library, $theme_copy_of_classy_library, $theme_path, 'js');
}
}
}
/**
* Checks for theme-specific equivalents of all Classy library-extends.
*
* Classy extends several core libraries with its own assets, these are
* defined in the `libraries-extend:` list in classy.info.yml. Classy adds
* additional assets to these libraries (e.g. when the `file/drupal.file`
* library loads, the assets of `classy/file` are loaded as well). For a theme
* to be properly decoupled from Classy's libraries, these core library
* extensions must become the responsibility of that theme.
*
* @param string $theme
* The theme being tested.
* @param string[] $extends_to_skip
* Classy library-extends excluded from the test.
*
* @dataProvider providerTestThemeAccountsForClassyExtensions
*/
public function testThemeAccountsForClassyExtensions($theme, array $extends_to_skip) {
$theme_path = $this->themeHandler->getTheme($theme)->getPath();
// Get a list of libraries overridden by the current theme. In a theme's
// info.yml file, these are the libraries listed in `libraries-override:`.
// They are libraries altered by the current theme.
$theme_library_overrides = $this->themeInitialization->getActiveThemeByName($theme)->getLibrariesOverride()[$theme_path] ?? [];
// Get a list of libraries extended by the current theme. In a theme's
// info.yml file, these are the libraries listed in `libraries-extend:`.
// The current theme adds additional files to these libraries.
$theme_extends = $this->themeHandler->getTheme($theme)->info['libraries-extend'] ?? [];
// Some Classy libraries extend core libraries (i.e. they are not standalone
// libraries. Rather, they extend the functionality of existing core
// libraries). These extensions that were implemented in Classy need to be
// accounted for in the current theme by either 1) The current theme
// extending the core library with local copy of the Classy library 2)
// Overriding the core library altogether.
// The following iterates through each library extended by Classy to confirm
// that the current theme accounts for these these extensions.
foreach ($this->classyLibrariesExtend as $library_extended => $info) {
if (in_array($library_extended, $extends_to_skip)) {
continue;
}
$extends_core_library = isset($theme_extends[$library_extended]);
$overrides_core_library = isset($theme_library_overrides[$library_extended]);
// Every core library extended by Classy must be extended or overridden by
// the current theme.
$this->assertTrue(($extends_core_library || $overrides_core_library), "$library_extended is extended by Classy and should be extended or overridden by $theme");
// If the core library is overridden, confirm that the override does not
// include any Classy assets.
if ($overrides_core_library) {
$overridden_with = $theme_library_overrides[$library_extended];