From 828ac7fe4f2e54319dca0159b3a94f5d19729ac6 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 27 Mar 2013 10:13:24 +0100 Subject: [PATCH 1/2] API Replaced SSViewer.custom_theme with SSViewer.theme_enabled Since we can't influence the setting of configuration values, we also can't set/unset the 'custom_theme' value based on which theme is set. This means the 'custom_theme' value goes stale, and we can't rely on it e.g. in FilesystemPublisher. The 'theme_enabled' toggle is a cleaner solution to the same problem, since the 'custom_theme' was really just a way to remember the original theme, while still disabling it. The toggle makes this more explicit, but also requires users of the 'theme' setting to check for it. --- admin/code/LeftAndMain.php | 7 +++---- docs/en/changelogs/3.1.0.md | 5 ++++- view/SSViewer.php | 36 ++++++++++++++++++++++++------------ view/ViewableData.php | 5 ++++- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index 3cca23b1b..d7ff2b56b 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -266,7 +266,7 @@ class LeftAndMain extends Controller implements PermissionProvider { // Use theme from the site config if(class_exists('SiteConfig') && ($config = SiteConfig::current_site_config()) && $config->Theme) { $theme = $config->Theme; - } elseif(Config::inst()->get('SSViewer', 'theme')) { + } elseif(Config::inst()->get('SSViewer', 'theme_enabled') && Config::inst()->get('SSViewer', 'theme')) { $theme = Config::inst()->get('SSViewer', 'theme'); } else { $theme = false; @@ -387,9 +387,8 @@ class LeftAndMain extends Controller implements PermissionProvider { $dummy = null; $this->extend('init', $dummy); - // The user's theme shouldn't affect the CMS, if, for example, they have replaced - // TableListField.ss or Form.ss. - Config::inst()->update('SSViewer', 'theme', null); + // The user's theme shouldn't affect the CMS, if, for example, they have replaced TableListField.ss or Form.ss. + Config::inst()->update('SSViewer', 'theme_enabled', false); } public function handleRequest(SS_HTTPRequest $request, DataModel $model = null) { diff --git a/docs/en/changelogs/3.1.0.md b/docs/en/changelogs/3.1.0.md index a5d27292d..4ba8abaad 100644 --- a/docs/en/changelogs/3.1.0.md +++ b/docs/en/changelogs/3.1.0.md @@ -427,4 +427,7 @@ you can enable those warnings and future-proof your code already. `YearlyTask` are deprecated, please extend from `BuildTask` or `CliController`, and invoke them in self-defined frequencies through Unix cronjobs etc. * `i18n::$common_locales` and `i18n::$common_languages` are now accessed via the Config API, and contain associative rather than indexed arrays. - Before: `array('de_DE' => array('German', 'Deutsch'))`, after: `array('de_DE' => array('name' => 'German', 'native' => 'Deutsch'))`. \ No newline at end of file + Before: `array('de_DE' => array('German', 'Deutsch'))`, after: `array('de_DE' => array('name' => 'German', 'native' => 'Deutsch'))`. + * `SSViewer::current_custom_theme()` has been replaced with the `SSViewer.theme_enabled` configuration setting. + Please use it to toggle theme behaviour rather than relying on the custom theme being set in the + (now deprecated) `SSViewer::set_theme()` call. \ No newline at end of file diff --git a/view/SSViewer.php b/view/SSViewer.php index e3a4d098f..88801d0da 100644 --- a/view/SSViewer.php +++ b/view/SSViewer.php @@ -572,15 +572,20 @@ class SSViewer { /** * @config - * @var string + * @var string The used "theme", which usually consists of templates, images and stylesheets. + * Only used when {@link $theme_enabled} is set to TRUE. */ - private static $current_theme = null; - + private static $theme = null; + /** * @config - * @var string + * @var string Use the theme. Set to FALSE in order to disable themes, + * which can be useful for scenarios where theme overrides are temporarily undesired, + * such as an administrative interface separate from the website theme. + * It retains the theme settings to be re-enabled, for example when a website content + * needs to be rendered from within this administrative interface. */ - private static $current_custom_theme = null; + private static $theme_enabled = true; /** * @var boolean @@ -603,9 +608,6 @@ class SSViewer { public static function set_theme($theme) { Deprecation::notice('3.2', 'Use the "SSViewer.theme" config setting instead'); Config::inst()->update('SSViewer', 'theme', $theme); - //Static publishing needs to have a theme set, otherwise it defaults to the content controller theme - if(!is_null($theme)) - Config::inst()->update('SSViewer', 'custom_theme', $theme); } /** @@ -655,8 +657,8 @@ class SSViewer { * @return string */ public static function current_custom_theme(){ - Deprecation::notice('3.2', 'Use the "SSViewer.theme" config setting instead'); - return Config::inst()->get('SSViewer', 'custom_theme'); + Deprecation::notice('3.2', 'Use the "SSViewer.theme" and "SSViewer.theme_enabled" config settings instead'); + return Config::inst()->get('SSViewer', 'theme_enabled') ? Config::inst()->get('SSViewer', 'theme') : null; } /** @@ -683,8 +685,13 @@ class SSViewer { if(!is_array($templateList) && substr((string) $templateList,-3) == '.ss') { $this->chosenTemplates['main'] = $templateList; } else { + if(Config::inst()->get('SSViewer', 'theme_enabled')) { + $theme = Config::inst()->get('SSViewer', 'theme'); + } else { + $theme = null; + } $this->chosenTemplates = SS_TemplateLoader::instance()->findTemplates( - $templateList, Config::inst()->get('SSViewer', 'theme') + $templateList, $theme ); } @@ -792,7 +799,12 @@ class SSViewer { */ public static function getTemplateFileByType($identifier, $type) { $loader = SS_TemplateLoader::instance(); - $found = $loader->findTemplates("$type/$identifier", Config::inst()->get('SSViewer', 'theme')); + if(Config::inst()->get('SSViewer', 'theme_enabled')) { + $theme = Config::inst()->get('SSViewer', 'theme'); + } else { + $theme = null; + } + $found = $loader->findTemplates("$type/$identifier", $theme); if ($found) { return $found['main']; diff --git a/view/ViewableData.php b/view/ViewableData.php index 895115069..7e3fcf341 100644 --- a/view/ViewableData.php +++ b/view/ViewableData.php @@ -522,7 +522,10 @@ class ViewableData extends Object implements IteratorAggregate { * @return string */ public function ThemeDir($subtheme = false) { - if($theme = Config::inst()->get('SSViewer', 'theme')) { + if( + Config::inst()->get('SSViewer', 'theme_enabled') + && $theme = Config::inst()->get('SSViewer', 'theme') + ) { return THEMES_DIR . "/$theme" . ($subtheme ? "_$subtheme" : null); } From 7121fc3f85fc51e7ad6c8b3e50fc09b5cc6b5905 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 27 Mar 2013 14:33:04 +0100 Subject: [PATCH 2/2] FIX Config isolation in Director::test() --- control/Director.php | 6 ++++-- view/SSViewer.php | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/control/Director.php b/control/Director.php index a1ef3b0e4..7f1d9f5b5 100644 --- a/control/Director.php +++ b/control/Director.php @@ -200,6 +200,8 @@ class Director implements TemplateGlobalProvider { public static function test($url, $postVars = null, $session = null, $httpMethod = null, $body = null, $headers = null, $cookies = null, &$request = null) { + Config::nest(); + // These are needed so that calling Director::test() doesnt muck with whoever is calling it. // Really, it's some inappropriate coupling and should be resolved by making less use of statics $oldStage = Versioned::current_stage(); @@ -217,7 +219,6 @@ class Director implements TemplateGlobalProvider { $existingCookies = isset($_COOKIE) ? $_COOKIE : array(); $existingServer = isset($_SERVER) ? $_SERVER : array(); - $existingCookieReportErrors = Config::inst()->get('Cookie', 'report_errors'); $existingRequirementsBackend = Requirements::backend(); Config::inst()->update('Cookie', 'report_errors', false); @@ -268,12 +269,13 @@ class Director implements TemplateGlobalProvider { $_COOKIE = $existingCookies; $_SERVER = $existingServer; - Config::inst()->update('Cookie', 'report_errors', $existingCookieReportErrors); Requirements::set_backend($existingRequirementsBackend); // These are needed so that calling Director::test() doesnt muck with whoever is calling it. // Really, it's some inappropriate coupling and should be resolved by making less use of statics Versioned::reading_stage($oldStage); + + Config::unnest(); return $result; } diff --git a/view/SSViewer.php b/view/SSViewer.php index 88801d0da..38e862d75 100644 --- a/view/SSViewer.php +++ b/view/SSViewer.php @@ -579,7 +579,7 @@ class SSViewer { /** * @config - * @var string Use the theme. Set to FALSE in order to disable themes, + * @var boolean Use the theme. Set to FALSE in order to disable themes, * which can be useful for scenarios where theme overrides are temporarily undesired, * such as an administrative interface separate from the website theme. * It retains the theme settings to be re-enabled, for example when a website content