From dc8d4ffe0bfa98043e23d24899f2fcf8c74cdd09 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 10 Feb 2015 23:13:55 +0000 Subject: [PATCH] API Refactor usages of HTMLText -> HTMLFragment API Revert hack in VirtualPage API Respect default_cast --- code/controllers/CMSMain.php | 14 +++++- code/model/SiteTree.php | 69 +++++++++++++++++--------- code/model/VirtualPage.php | 29 +++++------ templates/Includes/CMSMain_ListView.ss | 6 +-- templates/Includes/CMSMain_TreeView.ss | 46 ++++++++--------- tests/model/SiteTreeTest.php | 4 +- 6 files changed, 100 insertions(+), 68 deletions(-) diff --git a/code/controllers/CMSMain.php b/code/controllers/CMSMain.php index 0fa5bcf5..c610015a 100644 --- a/code/controllers/CMSMain.php +++ b/code/controllers/CMSMain.php @@ -74,6 +74,18 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr 'childfilter', ); + private static $casting = array( + 'TreeIsFiltered' => 'Boolean', + 'AddForm' => 'HTMLFragment', + 'LinkPages' => 'Text', + 'Link' => 'Text', + 'ListViewForm' => 'HTMLFragment', + 'ExtraTreeTools' => 'HTMLFragment', + 'SiteTreeHints' => 'HTMLFragment', + 'SecurityID' => 'Text', + 'SiteTreeAsUL' => 'HTMLFragment', + ); + public function init() { // set reading lang if(SiteTree::has_extension('Translatable') && !$this->getRequest()->isAjax()) { @@ -799,7 +811,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr $columns->setFieldCasting(array( 'Created' => 'DBDatetime->Ago', 'LastEdited' => 'DBDatetime->FormatFromSettings', - 'getTreeTitle' => 'HTMLText' + 'getTreeTitle' => 'HTMLFragment' )); $controller = $this; diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index 4d22fbb7..528ffff9 100755 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -127,7 +127,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid "MenuTitle" => "Varchar(100)", "Content" => "HTMLText", "MetaDescription" => "Text", - "ExtraMeta" => "HTMLText('meta, link')", + "ExtraMeta" => "HTMLFragment(['whitelist' => ['meta', 'link']])", "ShowInMenus" => "Boolean", "ShowInSearch" => "Boolean", "Sort" => "Int", @@ -156,11 +156,15 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid ); private static $casting = array( - "Breadcrumbs" => "HTMLText", + "Breadcrumbs" => "HTMLFragment", + "LastEdited" => "Datetime", + "Created" => "Datetime", 'Link' => 'Text', 'RelativeLink' => 'Text', 'AbsoluteLink' => 'Text', - 'TreeTitle' => 'HTMLText', + 'CMSEditLink' => 'Text', + 'TreeTitle' => 'HTMLFragment', + 'MetaTags' => 'HTMLFragment', ); private static $defaults = array( @@ -685,7 +689,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid * @param boolean $unlinked Whether to link page titles. * @param boolean|string $stopAtPageType ClassName of a page to stop the upwards traversal. * @param boolean $showHidden Include pages marked with the attribute ShowInMenus = 0 - * @return HTMLText The breadcrumb trail. + * @return string The breadcrumb trail. */ public function Breadcrumbs($maxDepth = 20, $unlinked = false, $stopAtPageType = false, $showHidden = false) { $pages = $this->getBreadcrumbItems($maxDepth, $stopAtPageType, $showHidden); @@ -762,6 +766,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid */ public function NestedTitle($level = 2, $separator = " - ") { $item = $this; + $parts = []; while($item && $level > 0) { $parts[] = $item->Title; $item = $item->Parent; @@ -1058,10 +1063,9 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid * @return SiteConfig */ public function getSiteConfig() { - - if($this->hasMethod('alternateSiteConfig')) { - $altConfig = $this->alternateSiteConfig(); - if($altConfig) return $altConfig; + $configs = $this->invokeWithExtensions('alternateSiteConfig'); + foreach(array_filter($configs) as $config) { + return $config; } return SiteConfig::current_site_config(); @@ -1190,6 +1194,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid // then see which ones the user has permission on $groupedByParent = array(); foreach($potentiallyInherited as $item) { + /** @var SiteTree $item */ if($item->ParentID) { if(!isset($groupedByParent[$item->ParentID])) $groupedByParent[$item->ParentID] = array(); $groupedByParent[$item->ParentID][] = $item->ID; @@ -1222,7 +1227,6 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid // Cache the results if(empty(self::$cache_permissions[$cacheKey])) self::$cache_permissions[$cacheKey] = array(); self::$cache_permissions[$cacheKey] = $combinedStageResult + self::$cache_permissions[$cacheKey]; - return $combinedStageResult; } else { return array(); @@ -1302,8 +1306,6 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid } else { $deletable = $editableIDs; } - } else { - $deletable = array(); } // Convert the array of deletable IDs into a map of the original IDs with true/false as the value @@ -1323,7 +1325,11 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid public function collateDescendants($condition, &$collator) { if($children = $this->Children()) { foreach($children as $item) { - if(eval("return $condition;")) $collator[] = $item; + + if(eval("return $condition;")) { + $collator[] = $item; + } + /** @var SiteTree $item */ $item->collateDescendants($condition, $collator); } return true; @@ -1339,23 +1345,29 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid * @return string The XHTML metatags */ public function MetaTags($includeTitle = true) { - $tags = ""; - if($includeTitle === true || $includeTitle == 'true') { - $tags .= "" . Convert::raw2xml($this->Title) . "\n"; + $tags = array(); + if($includeTitle && strtolower($includeTitle) != 'false') { + $tags[] = FormField::create_tag('title', array(), $this->obj('Title')->forTemplate()); } $generator = trim(Config::inst()->get('SiteTree', 'meta_generator')); if (!empty($generator)) { - $tags .= "\n"; + $tags[] = FormField::create_tag('meta', array( + 'name' => 'generator', + 'content' => $generator, + )); } $charset = Config::inst()->get('ContentNegotiator', 'encoding'); - $tags .= "\n"; + $tags[] = FormField::create_tag('meta', array( + 'http-equiv' => 'Content-Type', + 'content' => 'text/html; charset=' . $charset, + )); if($this->MetaDescription) { - $tags .= "MetaDescription) . "\" />\n"; - } - if($this->ExtraMeta) { - $tags .= $this->ExtraMeta . "\n"; + $tags[] = FormField::create_tag('meta', array( + 'name' => 'description', + 'content' => $this->MetaDescription, + )); } if(Permission::check('CMS_ACCESS_CMSMain') @@ -1363,8 +1375,19 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid && !$this instanceof ErrorPage && $this->ID > 0 ) { - $tags .= "ID}\" />\n"; - $tags .= "CMSEditLink() . "\" />\n"; + $tags[] = FormField::create_tag('meta', array( + 'name' => 'x-page-id', + 'content' => $this->obj('ID')->forTemplate(), + )); + $tags[] = FormField::create_tag('meta', array( + 'name' => 'x-cms-edit-link', + 'content' => $this->obj('CMSEditLink')->forTemplate(), + )); + } + + $tags = implode("\n", $tags); + if($this->ExtraMeta) { + $tags .= $this->obj('ExtraMeta')->forTemplate(); } $this->extend('MetaTags', $tags); diff --git a/code/model/VirtualPage.php b/code/model/VirtualPage.php index 4d8348ab..7105faf7 100644 --- a/code/model/VirtualPage.php +++ b/code/model/VirtualPage.php @@ -494,24 +494,21 @@ class VirtualPage_Controller extends Page_Controller { * @throws Exception Any error other than a 'no method' error. */ public function __call($method, $args) { - try { + // Check if we can safely call this method before passing it back + // to custom methods. + if($this->getExtraMethodConfig($method)) { return parent::__call($method, $args); - } catch (Exception $e) { - // Hack... detect exception type. We really should use exception subclasses. - // if the exception isn't a 'no method' error, rethrow it - if ($e->getCode() !== 2175) { - throw $e; - } - - $original = $this->copyContentFrom(); - $controller = ModelAsController::controller_for($original); - - // Ensure request/response data is available on virtual controller - $controller->setRequest($this->getRequest()); - $controller->response = $this->response; // @todo - replace with getter/setter in 3.3 - - return call_user_func_array(array($controller, $method), $args); } + + // Pass back to copied page + $original = $this->copyContentFrom(); + $controller = ModelAsController::controller_for($original); + + // Ensure request/response data is available on virtual controller + $controller->setRequest($this->getRequest()); + $controller->response = $this->response; // @todo - replace with getter/setter in 3.3 + + return call_user_func_array(array($controller, $method), $args); } } diff --git a/templates/Includes/CMSMain_ListView.ss b/templates/Includes/CMSMain_ListView.ss index 4694ec57..94ca14f8 100644 --- a/templates/Includes/CMSMain_ListView.ss +++ b/templates/Includes/CMSMain_ListView.ss @@ -8,17 +8,17 @@ <% if $TreeIsFiltered %>
<% _t('CMSMain.ListFiltered', 'Showing search results.') %> - + <% _t('CMSMain.TreeFilteredClear', 'Clear') %>
$ListViewForm -
+
<% else %> -
+
$ListViewForm
<% end_if %> diff --git a/templates/Includes/CMSMain_TreeView.ss b/templates/Includes/CMSMain_TreeView.ss index 00d93543..17a3bb4c 100644 --- a/templates/Includes/CMSMain_TreeView.ss +++ b/templates/Includes/CMSMain_TreeView.ss @@ -15,35 +15,35 @@ $ExtraTreeTools
+ data-url-tree="$LinkWithSearch($Link(getsubtree)).ATT" + data-url-savetreenode="$Link(savetreenode).ATT" + data-url-updatetreenodes="$Link(updatetreenodes).ATT" + data-url-addpage="{$LinkPageAdd('AddForm/?action_doAdd=1', 'ParentID=%s&PageType=%s').ATT}" + data-url-editpage="$LinkPageEdit('%s').ATT" + data-url-duplicate="{$Link('duplicate/%s').ATT}" + data-url-duplicatewithchildren="{$Link('duplicatewithchildren/%s').ATT}" + data-url-listview="{$Link('?view=list').ATT}" + data-hints="$SiteTreeHints.ATT" + data-childfilter="$Link('childfilter').ATT" + data-extra-params="SecurityID=$SecurityID.ATT"> $SiteTreeAsUL -
+
<% else %>
+ data-url-tree="$LinkWithSearch($Link(getsubtree)).ATT" + data-url-savetreenode="$Link(savetreenode).ATT" + data-url-updatetreenodes="$Link(updatetreenodes).ATT" + data-url-addpage="{$LinkPageAdd('AddForm/?action_doAdd=1', 'ParentID=%s&PageType=%s').ATT}" + data-url-editpage="$LinkPageEdit('%s').ATT" + data-url-duplicate="{$Link('duplicate/%s').ATT}" + data-url-duplicatewithchildren="{$Link('duplicatewithchildren/%s').ATT}" + data-url-listview="{$Link('?view=list').ATT}" + data-hints="$SiteTreeHints.ATT" + data-childfilter="$Link('childfilter').ATT" + data-extra-params="SecurityID=$SecurityID.ATT"> $SiteTreeAsUL
diff --git a/tests/model/SiteTreeTest.php b/tests/model/SiteTreeTest.php index c61dff0b..1667692c 100644 --- a/tests/model/SiteTreeTest.php +++ b/tests/model/SiteTreeTest.php @@ -1093,11 +1093,11 @@ class SiteTreeTest extends SapphireTest { // Test with title $meta = $page->MetaTags(); $charset = Config::inst()->get('ContentNegotiator', 'encoding'); - $this->assertContains('assertContains('assertContains('assertContains('assertContains('assertContains('', $meta); + $this->assertContains('assertContains('HTML & XML', $meta); // Test without title