From 38b122888df42599be0c1ab47333c356822fbb2f Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Thu, 3 Mar 2016 10:22:25 -0500 Subject: [PATCH] fix(events): the pagesetup event timing is more like 1.x In 1.x, the event was triggered just before the first view was rendered and it was assumed that some global state, particularly context, had been set by this time in page-specific code. In 2.0 this assumption is wrong because the page-specific code that sets state like context often runs *within* a resource view. Hence the event is triggered too early, before the page-specific state has been set up. Here we delay the `pagesetup` event if the first view is a resource view. This allows plugins to move all page-specific logic like context setting into a resource view with more confidence that that state will be available in their `pagesetup` event handlers. This likely fixes a class of undiscovered bugs like #9271 and #9322. Refs #8374 --- engine/classes/Elgg/ViewsService.php | 40 ++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/engine/classes/Elgg/ViewsService.php b/engine/classes/Elgg/ViewsService.php index b418a12bb35..d230253b7de 100644 --- a/engine/classes/Elgg/ViewsService.php +++ b/engine/classes/Elgg/ViewsService.php @@ -65,6 +65,11 @@ class ViewsService { * @var SystemCache|null This is set if the views are configured via cache */ private $cache; + + /** + * @var bool + */ + private $allow_delay_pagesetup = true; /** * Constructor @@ -275,11 +280,7 @@ public function renderView($view, array $vars = array(), $bypass = false, $viewt $view_orig = $view; - // Trigger the pagesetup event - if (!isset($GLOBALS['_ELGG']->pagesetupdone) && !empty($this->CONFIG->boot_complete)) { - $GLOBALS['_ELGG']->pagesetupdone = true; - _elgg_services()->events->trigger('pagesetup', 'system'); - } + $this->handlePageSetup($view); // Set up any extensions to the requested view if (isset($this->views->extensions[$view])) { @@ -328,6 +329,35 @@ protected function fileExists($path) { return $this->file_exists_cache[$path]; } + /** + * Trigger the system "pagesetup" event just before the 1st view rendering, or the 2nd if the 1st + * view starts with "resources/". + * + * We delay the pagesetup event if the first view is a resource view in order to allow plugins to + * move all page-specific logic like context setting into a resource view with more confidence + * that that state will be available in their pagesetup event handlers. See the commit message for + * more BG info. + * + * @param string $view View about to be rendered + * @return void + */ + private function handlePageSetup($view) { + if (isset($GLOBALS['_ELGG']->pagesetupdone) || empty($this->CONFIG->boot_complete)) { + return; + } + + // only first rendering gets an opportunity to delay + $allow_delay = $this->allow_delay_pagesetup; + $this->allow_delay_pagesetup = false; + + if ($allow_delay && (0 === strpos($view, 'resources/'))) { + return; + } + + $GLOBALS['_ELGG']->pagesetupdone = true; + _elgg_services()->events->trigger('pagesetup', 'system'); + } + /** * Includes view PHP or static file *