Merge pull request #6376 from lionel-rowe/ticket/16977

[ticket/16977] Fix cron-job img tag layout and accessibility

phpBB/config/default/container/services_cron.yml

@@ -6,6 +6,7 @@ services:
             - '@routing.helper'
             - '%core.root_path%'
             - '%core.php_ext%'
+            - '@template'
 
     cron.lock_db:
         class: phpbb\lock\db

phpBB/phpbb/controller/helper.php

@@ -363,8 +363,8 @@ class helper
 
 			if ($task)
 			{
-				$url = $task->get_url();
+				$cron_task_tag = $task->get_html_tag();
-				$this->template->assign_var('RUN_CRON_TASK', '<img src="' . $url . '" width="1" height="1" alt="cron" />');
+				$this->template->assign_var('RUN_CRON_TASK', $cron_task_tag);
 			}
 			else
 			{

phpBB/phpbb/cron/manager.php

@@ -59,6 +59,11 @@ class manager
 	 */
 	protected $php_ext;
 
+	/**
+	 * @var \phpbb\template\template
+	 */
+	protected $template;
+
 	/**
 	* Constructor. Loads all available tasks.
 	*
@@ -66,13 +71,15 @@ class manager
 	* @param helper $routing_helper Routing helper
 	* @param string $phpbb_root_path Relative path to phpBB root
 	* @param string $php_ext PHP file extension
+	* @param \phpbb\template\template $template
 	*/
-	public function __construct(ContainerInterface $phpbb_container, helper $routing_helper, $phpbb_root_path, $php_ext)
+	public function __construct(ContainerInterface $phpbb_container, helper $routing_helper, $phpbb_root_path, $php_ext, $template)
 	{
 		$this->phpbb_container = $phpbb_container;
 		$this->routing_helper = $routing_helper;
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->php_ext = $php_ext;
+		$this->template = $template;
 	}
 
 	/**
@@ -193,6 +200,6 @@ class manager
 	*/
 	public function wrap_task(\phpbb\cron\task\task $task)
 	{
-		return new wrapper($task, $this->routing_helper);
+		return new wrapper($task, $this->routing_helper, $this->template);
 	}
 }

phpBB/phpbb/cron/task/wrapper.php

@@ -31,6 +31,11 @@ class wrapper
 	 */
 	protected $task;
 
+	/**
+	 * @var \phpbb\template\template
+	 */
+	protected $template;
+
 	/**
 	* Constructor.
 	*
@@ -38,11 +43,13 @@ class wrapper
 	*
 	* @param task	$task				The cron task to wrap.
 	* @param helper	$routing_helper		Routing helper for route generation
+	* @param \phpbb\template\template	$template
 	*/
-	public function __construct(task $task, helper $routing_helper)
+	public function __construct(task $task, helper $routing_helper, $template)
 	{
 		$this->task = $task;
 		$this->routing_helper = $routing_helper;
+		$this->template = $template;
 	}
 
 	/**
@@ -92,6 +99,23 @@ class wrapper
 		return $this->routing_helper->route('phpbb_cron_run', $params);
 	}
 
+	/**
+	 * Returns HTML for an invisible `img` tag that can be displayed on page
+	 * load to trigger a request to the relevant cron task endpoint.
+	 *
+	 * @return string       HTML to render to trigger cron task
+	 */
+	public function get_html_tag()
+	{
+		$this->template->set_filenames([
+			'cron_html_tag' => 'cron.html',
+		]);
+
+		$this->template->assign_var('CRON_TASK_URL', $this->get_url());
+
+		return $this->template->assign_display('cron_html_tag');
+	}
+
 	/**
 	* Forwards all other method calls to the wrapped task implementation.
 	*

phpBB/styles/all/template/cron.html

@@ -0,0 +1,7 @@
+{#
+	Runs the cron task by triggering server request to the URL on load. `img` is
+	preferred over JS to ensure maximum compatibility. We use `class="sr-only"`
+	to hide visually and `aria-hidden="true"` to hide from screen-readers; using
+	`hidden` or `display: none` would prevent the task from running.
+#}
+<img class="sr-only" aria-hidden="true" src="{{ CRON_TASK_URL|e('html_attr') }}" width="1" height="1" alt="">

phpBB/styles/prosilver/template/overall_footer.html

@@ -68,7 +68,7 @@
 
 <div>
 	<a id="bottom" class="anchor" accesskey="z"></a>
-	<!-- IF not S_IS_BOT -->{RUN_CRON_TASK}<!-- ENDIF -->
+	{% if not S_IS_BOT %}{{ RUN_CRON_TASK }}{% endif %}
 </div>
 
 <script src="{T_JQUERY_LINK}"></script>

phpBB/viewforum.php

@@ -244,8 +244,8 @@ if (!$config['use_system_cron'])
 
 	if ($task->is_ready())
 	{
-		$url = $task->get_url();
+		$cron_task_tag = $task->get_html_tag();
-		$template->assign_var('RUN_CRON_TASK', '<img src="' . $url . '" width="1" height="1" alt="cron" />');
+		$template->assign_var('RUN_CRON_TASK', $cron_task_tag);
 	}
 	else
 	{
@@ -255,8 +255,8 @@ if (!$config['use_system_cron'])
 
 		if ($task->is_ready())
 		{
-			$url = $task->get_url();
+			$cron_task_tag = $task->get_html_tag();
-			$template->assign_var('RUN_CRON_TASK', '<img src="' . $url . '" width="1" height="1" alt="cron" />');
+			$template->assign_var('RUN_CRON_TASK', $cron_task_tag);
 		}
 	}
 }

tests/console/cron/cron_list_test.php

@@ -101,7 +101,7 @@ class phpbb_console_command_cron_list_test extends phpbb_test_case
 		$mock_container = new phpbb_mock_container_builder();
 		$mock_container->set('cron.task_collection', []);
 
-		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $pathEx);
+		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $pathEx, null);
 		$this->cron_manager->load_tasks($tasks);
 	}
 

tests/console/cron/run_test.php

@@ -77,7 +77,7 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case
 		$mock_container = new phpbb_mock_container_builder();
 		$mock_container->set('cron.task_collection', []);
 
-		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx);
+		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx, null);
 		$this->cron_manager->load_tasks($tasks);
 
 		$this->assertSame('0', $config['cron_lock']);
@@ -155,7 +155,7 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case
 		$mock_container = new phpbb_mock_container_builder();
 		$mock_container->set('cron.task_collection', []);
 
-		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx);
+		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx, null);
 		$this->cron_manager->load_tasks($tasks);
 
 		$command_tester = $this->get_command_tester();
@@ -202,7 +202,7 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case
 		$mock_container = new phpbb_mock_container_builder();
 		$mock_container->set('cron.task_collection', []);
 
-		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx);
+		$this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx, null);
 		$this->cron_manager->load_tasks($tasks);
 
 		$command_tester = $this->get_command_tester();

tests/controller/common_helper_route.php

@@ -211,7 +211,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -274,7 +274,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -337,7 +337,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -400,7 +400,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -463,7 +463,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -526,7 +526,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -586,7 +586,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -649,7 +649,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,
@@ -701,7 +701,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_
 			$this->auth,
 			$this->cache,
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php', null),
 			$this->db,
 			$this->dispatcher,
 			$this->language,

tests/cron/manager_test.php

@@ -107,7 +107,7 @@ class phpbb_cron_manager_test extends \phpbb_test_case
 		$mock_container = new phpbb_mock_container_builder();
 		$mock_container->set('cron.task_collection', []);
 
-		$cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx);
+		$cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx, null);
 		$cron_manager->load_tasks($tasks);
 
 		return $cron_manager;

tests/pagination/pagination_test.php

@@ -66,7 +66,7 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
 			new \phpbb\auth\auth(),
 			new \phpbb\cache\driver\dummy(),
 			$this->config,
-			new \phpbb\cron\manager($mock_container, $this->routing_helper, '', 'php'),
+			new \phpbb\cron\manager($mock_container, $this->routing_helper, '', 'php', null),
 			$db,
 			new phpbb_mock_event_dispatcher(),
 			new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)),