From f09170984736fc529374a73f655ca51a482dbb26 Mon Sep 17 00:00:00 2001 From: Cameron Date: Tue, 19 Jan 2021 09:50:02 -0800 Subject: [PATCH] Issue #4352 - Canonical added for page, forum and download. Replaced some e_PAGETITLE with e107::title() --- e107_handlers/e107_class.php | 24 +++++-- e107_handlers/e_parse_class.php | 2 +- .../download/handlers/download_class.php | 64 ++++++++++--------- e107_plugins/forum/forum.php | 4 ++ e107_plugins/forum/forum_post.php | 1 + e107_plugins/forum/forum_viewforum.php | 6 +- e107_plugins/forum/forum_viewtopic.php | 2 +- e107_tests/tests/unit/e107Test.php | 12 ++++ e107_web/css/e107.css | 5 +- page.php | 63 ++++++++++++++---- 10 files changed, 128 insertions(+), 55 deletions(-) diff --git a/e107_handlers/e107_class.php b/e107_handlers/e107_class.php index d9ccd12c2..e22e5a590 100644 --- a/e107_handlers/e107_class.php +++ b/e107_handlers/e107_class.php @@ -3771,19 +3771,35 @@ class e107 /** * Quick method to set alias - uses e107::url format. - * @param string $plugin - * @param null $key + * @param string $plugin if empty will return the last assigned canonical url. + * @param string|array $key * @param array $row */ public static function canonical($plugin = '', $key = 'index', $row = array()) { - if($url = e107::url($plugin, $key, $row, array('mode' => 'full'))) + + $alreadyDone = e107::getRegistry('core/e107/canonical'); + + if(empty($plugin)) + { + return $alreadyDone; + } + + if(empty($alreadyDone) && $url = e107::url($plugin, $key, $row, array('mode' => 'full'))) { self::getJs()->addLink(array('rel'=>"canonical", "href" => $url)); + e107::setRegistry('core/e107/canonical', $url); } + + if(!empty($alreadyDone)) + { + $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2); + $url = "More than one canonical was set: ".print_a($backtrace[1], true); + } + if(deftrue('e_DEBUG_CANONICAL')) { - self::getMessage()->addInfo("Debug Canonical URL: ".$url); + self::getMessage()->addInfo("Debug Canonical URL: ".$url.""); } } diff --git a/e107_handlers/e_parse_class.php b/e107_handlers/e_parse_class.php index daa45cc14..c08caf559 100644 --- a/e107_handlers/e_parse_class.php +++ b/e107_handlers/e_parse_class.php @@ -1899,7 +1899,7 @@ class e_parse * @param string $text * @return mixed|string */ - private function ampEncode($text = '') + public function ampEncode($text = '') { // Fix any left-over '&' diff --git a/e107_plugins/download/handlers/download_class.php b/e107_plugins/download/handlers/download_class.php index c5300287c..2d24ee61e 100644 --- a/e107_plugins/download/handlers/download_class.php +++ b/e107_plugins/download/handlers/download_class.php @@ -164,37 +164,34 @@ class download $pref = e107::getPref(); - - - //if($this->qry['action'] == 'maincats') - // { - // - // } - - if($this->qry['action'] == 'list') + switch($this->qry['action']) { - $this->loadList(); + case 'maincats': + e107::canonical('download', 'index'); + break; + + case "list": + $this->loadList(); + break; + + case "view": + $this->loadView(); + break; + + case "report": + if(check_class($pref['download_reportbroken'])) + { + $this->loadReport(); + } + break; + /* + case 'mirror': + case 'error': + + break; */ + } - if($this->qry['action'] == 'view') - { - $this->loadView(); - } - - if ($this->qry['action'] == "report" && check_class($pref['download_reportbroken'])) - { - $this->loadReport(); - } - - //if($this->qry['action'] == 'mirror') - // { - // - // } - - // if($this->qry['action'] == 'error') - // { - - // } } @@ -428,11 +425,14 @@ class download { if($dlrow = $this->getCategory($this->qry['id'])) { - define("e_PAGETITLE", LAN_PLUGIN_DOWNLOAD_NAME." / ".$dlrow['download_category_name']); + e107::title(LAN_PLUGIN_DOWNLOAD_NAME." / ".$dlrow['download_category_name']); + e107::canonical('download', 'category', $dlrow); + // define("e_PAGETITLE", LAN_PLUGIN_DOWNLOAD_NAME." / ".$dlrow['download_category_name']); } else { // No access to this category - define("e_PAGETITLE", LAN_PLUGIN_DOWNLOAD_NAME); + e107::title(LAN_PLUGIN_DOWNLOAD_NAME); + //define("e_PAGETITLE", LAN_PLUGIN_DOWNLOAD_NAME); } return null; @@ -559,7 +559,9 @@ class download $DL_TITLE = e107::getParser()->parseTemplate($this->template['pagetitle'], true, $sc); - define("e_PAGETITLE", $DL_TITLE); + // define("e_PAGETITLE", $DL_TITLE); + e107::title($DL_TITLE); + e107::canonical('download', 'item', $dlrow); return null; } diff --git a/e107_plugins/forum/forum.php b/e107_plugins/forum/forum.php index 565441d02..0c340229a 100644 --- a/e107_plugins/forum/forum.php +++ b/e107_plugins/forum/forum.php @@ -90,6 +90,10 @@ if(isset($_GET['f'])) break; } } +else +{ + e107::canonical('forum'); +} diff --git a/e107_plugins/forum/forum_post.php b/e107_plugins/forum/forum_post.php index 01e5cfd08..7b2ed2321 100644 --- a/e107_plugins/forum/forum_post.php +++ b/e107_plugins/forum/forum_post.php @@ -41,6 +41,7 @@ if (!e107::isInstalled('forum')) //e107::lan('forum','English_front'); e107::lan('forum', "front", true); e107::css('forum','forum.css'); +// e107::canonical('forum', 'post'); diff --git a/e107_plugins/forum/forum_viewforum.php b/e107_plugins/forum/forum_viewforum.php index 4eb4bd759..02394eae9 100644 --- a/e107_plugins/forum/forum_viewforum.php +++ b/e107_plugins/forum/forum_viewforum.php @@ -56,7 +56,7 @@ echo __FILE__ . ' Line: ' . __LINE__; exit; } - $url = e107::url('forum', 'index', 'full'); + $url = e107::url('forum', 'index', null, ['mode'=>'full']); e107::getRedirect()->go($url); //header('Location:'.e107::getUrl()->create('forum/forum/main', array(), 'full=1&encode=0')); exit; @@ -100,7 +100,7 @@ { // header('Location:'.e107::getUrl()->create('forum/forum/main')); - $url = e107::url('forum', 'index', 'full'); + $url = e107::url('forum', 'index', null, ['mode'=>'full']); if(E107_DEBUG_LEVEL > 0) { @@ -118,6 +118,8 @@ } $forumInfo = $forum->forumGet($forumId); + e107::canonical('forum', 'forum', $forumInfo); + $forumSCvars = array(); //----$threadsViewed = $forum->threadGetUserViewed(); diff --git a/e107_plugins/forum/forum_viewtopic.php b/e107_plugins/forum/forum_viewtopic.php index 17e8ccd29..fddfc7edd 100644 --- a/e107_plugins/forum/forum_viewtopic.php +++ b/e107_plugins/forum/forum_viewtopic.php @@ -627,7 +627,7 @@ else unset($ret); } - +e107::canonical('forum', 'topic', $thread->threadInfo); require_once (HEADERF); diff --git a/e107_tests/tests/unit/e107Test.php b/e107_tests/tests/unit/e107Test.php index e27494701..06d4679b6 100644 --- a/e107_tests/tests/unit/e107Test.php +++ b/e107_tests/tests/unit/e107Test.php @@ -967,6 +967,18 @@ class e107Test extends \Codeception\Test\Unit } + public function testCanonical() + { + $e107 = $this->e107; + + $e107::canonical('news'); + + $result = $e107::canonical(); + $this->assertSame("https://localhost/e107/news", $result); + + } + + public function testUrl() { diff --git a/e107_web/css/e107.css b/e107_web/css/e107.css index ec947f86f..15a1eca61 100644 --- a/e107_web/css/e107.css +++ b/e107_web/css/e107.css @@ -105,12 +105,13 @@ a.cpage-np.current { text-decoration: none; } /* validate result inline message */ .validate-result {} -/* message text (overall) */ +/* message text (overall) - Too generic - creates conflicts */ +/* .warning { color: #FF6600 } .success { color: green; } .error { color: #A94442; } .info {} -.required { color:#A94442; } +.required { color:#A94442; }*/ /******** Tooltip info */ .e-info {} diff --git a/page.php b/page.php index c525d6569..19af2b0e3 100644 --- a/page.php +++ b/page.php @@ -19,6 +19,7 @@ if(!e_QUERY) $e107CorePage->setRequest('listBooks'); $e107CorePage->listBooks(); + e107::canonical('page/list/index'); require_once(HEADERF); e107::getRender()->tablerender($e107CorePage->pageOutput['caption'], $e107CorePage->pageOutput['text'], "cpage-full-list"); @@ -36,8 +37,9 @@ if(!e_QUERY) } elseif(vartrue($_GET['bk'])) // List Chapters within a specific Book { - $e107CorePage->setRequest('listChapters'); - $e107CorePage->listChapters($_GET['bk']); + $id = $e107CorePage->setRequest('listChapters'); + $e107CorePage->listChapters($id); + e107::canonical('page/book/index', $e107CorePage->getChapterData($id)); require_once(HEADERF); e107::getRender()->tablerender($e107CorePage->pageOutput['caption'], $e107CorePage->pageOutput['text'], 'cpage-chapter-list'); @@ -46,9 +48,10 @@ elseif(vartrue($_GET['bk'])) // List Chapters within a specific Book } elseif(vartrue($_GET['ch'])) // List Pages within a specific Chapter { - $e107CorePage->setRequest('listPages'); - $e107CorePage->listPages($_GET['ch']); - + $id = $e107CorePage->setRequest('listPages'); + $e107CorePage->listPages($id); + e107::canonical('page/chapter/index', $e107CorePage->getChapterData($id)); + unset($row); require_once(HEADERF); e107::getRender()->tablerender($e107CorePage->pageOutput['caption'], $e107CorePage->pageOutput['text'], 'cpage-page-list'); require_once(FOOTERF); @@ -60,6 +63,7 @@ else $e107CorePage->processViewPage(); $e107CorePage->setPage(); + e107::canonical('page/view', $e107CorePage->page); require_once(HEADERF); $ns = e107::getRender(); @@ -164,7 +168,6 @@ class pageClass } - // XXX temporary solution - upcoming page rewrite public function setRequest($request) { switch ($request) @@ -187,9 +190,33 @@ class pageClass break; } e107::setRegistry('core/page/request', array('action' => $request, 'id' => $id)); + + return $id; } - + + + /** + * @param $chapter + * @return array|mixed + */ + public function getChapterData($chapter) + { + if(!isset($this->chapterData[$chapter])) + { + return array(); + } + + $row = $this->chapterData[$chapter]; + + $row['book_sef'] = $this->getSef($row['chapter_id']); + $row['book_id'] = (int) $row['chapter_parent']; + $row['book_name'] = $this->getName($row['chapter_parent']); + $row['book_icon'] = $this->getIcon($row['chapter_parent']); + $row['book_description'] = $this->getDescription($row['chapter_parent']); + + return $row; + } private function getSef($chapter) { @@ -311,14 +338,21 @@ class pageClass { $layout = $brow['chapter_template']; } + + if($this->displayAllMode === true) { $layout = e107::getPref('listBooksTemplate'); } - - - + + if(empty($layout) && ADMIN) + { + e107::getMessage()->addError("Admin Only Message: The chapter template is empty. Reverting to 'default'."); // NO LAN + $layout = 'default'; + } + + $tml = e107::getCoreTemplate('chapter','', true, true); // always merge $error = array('listChapters' => array('start'=>"Chapter template not found: ".$layout)); @@ -454,7 +488,7 @@ class pageClass 'book_id' => $row['chapter_parent'] ); - + //print_a($this->chapterData); $tml = e107::getCoreTemplate('chapter','', true, true); // always merge @@ -692,8 +726,9 @@ class pageClass $this->batch->setVars($this->page); $this->batch->breadcrumb(); - - define('e_PAGETITLE', eHelper::formatMetaTitle($this->page['page_title'])); + + e107::title(eHelper::formatMetaTitle($this->page['page_title'])); + if($this->page['page_metadscr']) define('META_DESCRIPTION', eHelper::formatMetaDescription($this->page['page_metadscr'])); if(!empty($this->page['page_metakeys'])) { @@ -1098,4 +1133,4 @@ class pageClass } } -?> +