From 8c982c7aa0d2861d03e7b216c9be32b0690d7f6c Mon Sep 17 00:00:00 2001 From: lionel-rowe Date: Fri, 1 Apr 2022 22:54:33 +0100 Subject: [PATCH 1/3] [ticket/16977] Fix cron-job img tag layout and accessibility PHPBB3-16977 --- phpBB/phpbb/controller/helper.php | 2 +- phpBB/viewforum.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 3262e6bbc4..d4eef1c374 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -364,7 +364,7 @@ class helper if ($task) { $url = $task->get_url(); - $this->template->assign_var('RUN_CRON_TASK', 'cron'); + $this->template->assign_var('RUN_CRON_TASK', ''); } else { diff --git a/phpBB/viewforum.php b/phpBB/viewforum.php index f070e0400f..4eba9bde25 100644 --- a/phpBB/viewforum.php +++ b/phpBB/viewforum.php @@ -245,7 +245,7 @@ if (!$config['use_system_cron']) if ($task->is_ready()) { $url = $task->get_url(); - $template->assign_var('RUN_CRON_TASK', 'cron'); + $template->assign_var('RUN_CRON_TASK', ''); } else { @@ -256,7 +256,7 @@ if (!$config['use_system_cron']) if ($task->is_ready()) { $url = $task->get_url(); - $template->assign_var('RUN_CRON_TASK', 'cron'); + $template->assign_var('RUN_CRON_TASK', ''); } } } From 87c1e631efba4569cceb26d9287567fe96c9ae92 Mon Sep 17 00:00:00 2001 From: lionel-rowe Date: Sat, 2 Apr 2022 18:10:53 +0100 Subject: [PATCH 2/3] [ticket/16977] Move HTML rendering logic to template PHPBB3-16977 --- .../default/container/services_cron.yml | 1 + phpBB/phpbb/controller/helper.php | 4 +-- phpBB/phpbb/cron/manager.php | 11 ++++++-- phpBB/phpbb/cron/task/wrapper.php | 26 ++++++++++++++++++- phpBB/styles/all/template/cron.html | 7 +++++ .../prosilver/template/overall_footer.html | 2 +- phpBB/viewforum.php | 8 +++--- 7 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 phpBB/styles/all/template/cron.html diff --git a/phpBB/config/default/container/services_cron.yml b/phpBB/config/default/container/services_cron.yml index 44030c1750..0117339e29 100644 --- a/phpBB/config/default/container/services_cron.yml +++ b/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 diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index d4eef1c374..9c564f55ae 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -363,8 +363,8 @@ class helper if ($task) { - $url = $task->get_url(); - $this->template->assign_var('RUN_CRON_TASK', ''); + $cron_task_tag = $task->get_html_tag(); + $this->template->assign_var('RUN_CRON_TASK', $cron_task_tag); } else { diff --git a/phpBB/phpbb/cron/manager.php b/phpBB/phpbb/cron/manager.php index a8fdb13857..f717f23a6c 100644 --- a/phpBB/phpbb/cron/manager.php +++ b/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, $this->phpbb_root_path, $this->php_ext); + return new wrapper($task, $this->routing_helper, $this->phpbb_root_path, $this->php_ext, $this->template); } } diff --git a/phpBB/phpbb/cron/task/wrapper.php b/phpBB/phpbb/cron/task/wrapper.php index 4dc3a7fb95..58de195acf 100644 --- a/phpBB/phpbb/cron/task/wrapper.php +++ b/phpBB/phpbb/cron/task/wrapper.php @@ -41,6 +41,11 @@ class wrapper */ protected $php_ext; + /** + * @var \phpbb\template\template + */ + protected $template; + /** * Constructor. * @@ -50,13 +55,15 @@ class wrapper * @param helper $routing_helper Routing helper for route generation * @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(task $task, helper $routing_helper, $phpbb_root_path, $php_ext) + public function __construct(task $task, helper $routing_helper, $phpbb_root_path, $php_ext, $template) { $this->task = $task; $this->routing_helper = $routing_helper; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; + $this->template = $template; } /** @@ -105,6 +112,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. * diff --git a/phpBB/styles/all/template/cron.html b/phpBB/styles/all/template/cron.html new file mode 100644 index 0000000000..5e4c901263 --- /dev/null +++ b/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. +#} + diff --git a/phpBB/styles/prosilver/template/overall_footer.html b/phpBB/styles/prosilver/template/overall_footer.html index c07c1e1de2..5d872e2415 100644 --- a/phpBB/styles/prosilver/template/overall_footer.html +++ b/phpBB/styles/prosilver/template/overall_footer.html @@ -60,7 +60,7 @@
- {RUN_CRON_TASK} + {% if not S_IS_BOT %}{{ RUN_CRON_TASK }}{% endif %}
diff --git a/phpBB/viewforum.php b/phpBB/viewforum.php index 4eba9bde25..7f5cdd34da 100644 --- a/phpBB/viewforum.php +++ b/phpBB/viewforum.php @@ -244,8 +244,8 @@ if (!$config['use_system_cron']) if ($task->is_ready()) { - $url = $task->get_url(); - $template->assign_var('RUN_CRON_TASK', ''); + $cron_task_tag = $task->get_html_tag(); + $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(); - $template->assign_var('RUN_CRON_TASK', ''); + $cron_task_tag = $task->get_html_tag(); + $template->assign_var('RUN_CRON_TASK', $cron_task_tag); } } } From e53102feadac5f4729b868f9c023021aa46581b0 Mon Sep 17 00:00:00 2001 From: lionel-rowe Date: Sun, 3 Apr 2022 20:46:44 +0100 Subject: [PATCH 3/3] [ticket/16977] Fix broken tests PHPBB3-16977 --- tests/console/cron/cron_list_test.php | 2 +- tests/console/cron/run_test.php | 6 +++--- tests/controller/common_helper_route.php | 18 +++++++++--------- tests/cron/manager_test.php | 2 +- tests/pagination/pagination_test.php | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/console/cron/cron_list_test.php b/tests/console/cron/cron_list_test.php index 30e93a7850..4694451ab3 100644 --- a/tests/console/cron/cron_list_test.php +++ b/tests/console/cron/cron_list_test.php @@ -105,7 +105,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); } diff --git a/tests/console/cron/run_test.php b/tests/console/cron/run_test.php index 205ff821b5..b5d7656ec0 100644 --- a/tests/console/cron/run_test.php +++ b/tests/console/cron/run_test.php @@ -81,7 +81,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']); @@ -160,7 +160,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(); @@ -208,7 +208,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(); diff --git a/tests/controller/common_helper_route.php b/tests/controller/common_helper_route.php index b565f17f1a..be755593c7 100644 --- a/tests/controller/common_helper_route.php +++ b/tests/controller/common_helper_route.php @@ -212,7 +212,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, @@ -275,7 +275,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, @@ -338,7 +338,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, @@ -401,7 +401,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, @@ -464,7 +464,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, @@ -527,7 +527,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, @@ -587,7 +587,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, @@ -650,7 +650,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, diff --git a/tests/cron/manager_test.php b/tests/cron/manager_test.php index ea8ebc05dc..99bcf4cc59 100644 --- a/tests/cron/manager_test.php +++ b/tests/cron/manager_test.php @@ -105,7 +105,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; diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index aa192a7040..58557bca52 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -70,7 +70,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)),