Merge pull request #6632 from iMattPro/ticket/17333

[ticket/17333] UX Enhancements for Web Push Subscribing

phpBB/includes/acp/acp_board.php

@@ -494,6 +494,8 @@ class acp_board
 						'webpush_enable'			=> ['lang' => 'WEBPUSH_ENABLE', 'validate' => 'bool', 'type' => 'custom', 'method' => 'webpush_enable', 'explain' => true],
 						'webpush_vapid_public'		=> ['lang' => 'WEBPUSH_VAPID_PUBLIC', 'validate' => 'string', 'type' => 'text:25:255', 'explain' => true],
 						'webpush_vapid_private'		=> ['lang' => 'WEBPUSH_VAPID_PRIVATE', 'validate' => 'string', 'type' => 'password:25:255', 'explain' => true],
+						'webpush_method_default_enable'	=> ['lang' => 'WEBPUSH_METHOD_DEFAULT_ENABLE', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true],
+						'webpush_dropdown_subscribe'	=> ['lang' => 'WEBPUSH_DROPDOWN_SUBSCRIBE', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true],
 
 						'legend3'				=> 'ACP_SUBMIT_CHANGES',
 					],

phpBB/includes/functions.php

@@ -3735,6 +3735,9 @@ function page_header($page_title = '', $display_online_list = false, $item_id =
 		$timezone_name = $user->lang['timezones'][$timezone_name];
 	}
 
+	/** @var \phpbb\controller\helper $controller_helper */
+	$controller_helper = $phpbb_container->get('controller.helper');
+
 	// Output the notifications
 	$notifications = false;
 	if ($config['load_notifications'] && $config['allow_board_notifications'] && $user->data['user_id'] != ANONYMOUS && $user->data['user_type'] != USER_IGNORE)
@@ -3751,10 +3754,22 @@ function page_header($page_title = '', $display_online_list = false, $item_id =
 		{
 			$template->assign_block_vars('notifications', $notification->prepare_for_display());
 		}
+
+		// Assign web push template vars globally (if not done already by ucp_notifications) for the dropdown subscribe button
+		if ($config['webpush_enable'] && $config['webpush_dropdown_subscribe']
+			&& $template->retrieve_var('NOTIFICATIONS_WEBPUSH_ENABLE') === null)
+		{
+			$methods = $phpbb_notifications->get_subscription_methods();
+			$webpush = $methods['notification.method.webpush'] ?? null;
+
+			if ($webpush)
+			{
+				$form_helper = $phpbb_container->get('form_helper');
+				$template->assign_vars($webpush['method']->get_ucp_template_data($controller_helper, $form_helper));
+			}
+		}
 	}
 
-	/** @var \phpbb\controller\helper $controller_helper */
-	$controller_helper = $phpbb_container->get('controller.helper');
 	$notification_mark_hash = generate_link_hash('mark_all_notifications_read');
 
 	$phpbb_version_parts = explode('.', PHPBB_VERSION, 3);

phpBB/install/schemas/schema_data.sql

@@ -329,6 +329,8 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\backup\con
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('webpush_enable', '0');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('webpush_vapid_public', '');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('webpush_vapid_private', '');
+INSERT INTO phpbb_config (config_name, config_value) VALUES ('webpush_method_default_enable', '1');
+INSERT INTO phpbb_config (config_name, config_value) VALUES ('webpush_dropdown_subscribe', '1');
 
 INSERT INTO phpbb_config (config_name, config_value, is_dynamic) VALUES ('cache_last_gc', '0', 1);
 INSERT INTO phpbb_config (config_name, config_value, is_dynamic) VALUES ('cron_lock', '0', 1);

phpBB/language/en/acp/board.php

@@ -590,6 +590,10 @@ $lang = array_merge($lang, [
 	'WEBPUSH_VAPID_PUBLIC_EXPLAIN'	=> 'The Voluntary Application Server Identification (VAPID) public key is shared to authenticate push messages from your site.<br><em><strong>Caution:</strong> Modifying the VAPID public key will automatically render all Web Push subscriptions invalid.</em>',
 	'WEBPUSH_VAPID_PRIVATE'			=> 'Server identification private key',
 	'WEBPUSH_VAPID_PRIVATE_EXPLAIN'	=> 'The Voluntary Application Server Identification (VAPID) private key is used to generate authenticated push messages dispatched from your site. The VAPID private key <strong>must</strong> form a valid public-private key pair alongside the VAPID public key.<br><em><strong>Caution:</strong> Modifying the VAPID private key will automatically render all Web Push subscriptions invalid.</em>',
+	'WEBPUSH_METHOD_DEFAULT_ENABLE'	=> 'Enable all user-based web push notification options by default',
+	'WEBPUSH_METHOD_DEFAULT_ENABLE_EXPLAIN'	=> 'When this setting is enabled, users who subscribe and allow browser notifications will start receiving them automatically. Users only need to visit the UCP Notification settings to disable any unwanted notifications.<br><br>If this setting is disabled, users will not receive any notifications, even if they have subscribed, until they visit the UCP Notification settings to enable the specific notification options they wish to receive.',
+	'WEBPUSH_DROPDOWN_SUBSCRIBE'	=> 'Show “Subscribe” button in notification dropdown',
+	'WEBPUSH_DROPDOWN_SUBSCRIBE_EXPLAIN'	=> 'Display a “Subscribe” button in the Notification dropdown, allowing users to easily subscribe to push notifications from anywhere in the forum.',
 ]);
 
 // Jabber settings

phpBB/language/en/common.php

@@ -512,6 +512,9 @@ $lang = array_merge($lang, array(
 	),
 	'NOTIFY_ADMIN'				=> 'Please notify the board administrator or webmaster.',
 	'NOTIFY_ADMIN_EMAIL'		=> 'Please notify the board administrator or webmaster: <a href="mailto:%1$s">%1$s</a>',
+	'NOTIFY_WEB_PUSH_ENABLE'	=> 'Enable Web Push notifications',
+	'NOTIFY_WEB_PUSH_SUBSCRIBE'	=> 'Subscribe',
+	'NOTIFY_WEB_PUSH_SUBSCRIBED'=> 'Subscribed',
 	'NO_ACCESS_ATTACHMENT'		=> 'You are not allowed to access this file.',
 	'NO_ACTION'					=> 'No action specified.',
 	'NO_ADMINISTRATORS'			=> 'There are no administrators.',

phpBB/phpbb/db/migration/data/v400/add_webpush_options.php

@@ -0,0 +1,47 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ * For full copyright and license information, please see
+ * the docs/CREDITS.txt file.
+ *
+ */
+
+namespace phpbb\db\migration\data\v400;
+
+use phpbb\db\migration\migration;
+
+class add_webpush_options extends migration
+{
+	public static function depends_on(): array
+	{
+		return [
+			'\phpbb\db\migration\data\v400\add_webpush',
+		];
+	}
+
+	public function effectively_installed(): bool
+	{
+		return $this->config->offsetExists('webpush_method_default_enable') || $this->config->offsetExists('webpush_dropdown_subscribe');
+	}
+
+	public function update_data(): array
+	{
+		return [
+			['config.add', ['webpush_method_default_enable', true]],
+			['config.add', ['webpush_dropdown_subscribe', true]],
+		];
+	}
+
+	public function revert_data(): array
+	{
+		return [
+			['config.remove', ['webpush_method_default_enable']],
+			['config.remove', ['webpush_dropdown_subscribe']],
+		];
+	}
+}

phpBB/phpbb/notification/method/webpush.php

@@ -48,6 +48,9 @@ class webpush extends messenger_base implements extended_method_interface
 	/** @var string Notification push subscriptions table */
 	protected $push_subscriptions_table;
 
+	/** @var int Fallback size for padding if endpoint is mozilla, see https://github.com/web-push-libs/web-push-php/issues/108#issuecomment-2133477054 */
+	const MOZILLA_FALLBACK_PADDING = 2820;
+
 	/**
 	 * Notification Method Web Push constructor
 	 *
@@ -91,6 +94,14 @@ class webpush extends messenger_base implements extended_method_interface
 			&& !empty($this->config['webpush_vapid_public']) && !empty($this->config['webpush_vapid_private']);
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	public function is_enabled_by_default()
+	{
+		return (bool) $this->config['webpush_method_default_enable'];
+	}
+
 	/**
 	* {@inheritdoc}
 	*/
@@ -218,6 +229,7 @@ class webpush extends messenger_base implements extended_method_interface
 			{
 				try
 				{
+					$this->set_endpoint_padding($web_push, $subscription['endpoint']);
 					$push_subscription = Subscription::create([
 						'endpoint'			=> $subscription['endpoint'],
 						'keys'				=> [
@@ -348,7 +360,7 @@ class webpush extends messenger_base implements extended_method_interface
 		}
 
 		return [
-			'NOTIFICATIONS_WEBPUSH_ENABLE'	=> true,
+			'NOTIFICATIONS_WEBPUSH_ENABLE'	=> $this->config['webpush_dropdown_subscribe'] || stripos($this->user->page['page'], 'notification_options'),
 			'U_WEBPUSH_SUBSCRIBE'			=> $controller_helper->route('phpbb_ucp_push_subscribe_controller'),
 			'U_WEBPUSH_UNSUBSCRIBE'			=> $controller_helper->route('phpbb_ucp_push_unsubscribe_controller'),
 			'VAPID_PUBLIC_KEY'				=> $this->config['webpush_vapid_public'],
@@ -430,4 +442,27 @@ class webpush extends messenger_base implements extended_method_interface
 
 		$this->remove_subscriptions($remove_subscriptions);
 	}
+
+	/**
+	 * Set web push padding for endpoint
+	 *
+	 * @param \Minishlink\WebPush\WebPush $web_push
+	 * @param string $endpoint
+	 *
+	 * @return void
+	 */
+	protected function set_endpoint_padding(\Minishlink\WebPush\WebPush $web_push, string $endpoint): void
+	{
+		if (str_contains($endpoint, 'mozilla.com') || str_contains($endpoint, 'mozaws.net'))
+		{
+			try
+			{
+				$web_push->setAutomaticPadding(self::MOZILLA_FALLBACK_PADDING);
+			}
+			catch (\Exception)
+			{
+				// This shouldn't happen since we won't pass padding length outside limits
+			}
+		}
+	}
 }

phpBB/styles/prosilver/template/notification_dropdown.html

@@ -44,6 +44,13 @@
 		<div class="footer">
 			<a href="{U_VIEW_ALL_NOTIFICATIONS}"><span>{L_SEE_ALL}</span></a>
 		</div>
+		{% if NOTIFICATIONS_WEBPUSH_ENABLE and notification_types is not defined %}
+			<div class="notification-dropdown-footer">
+				<span class="ellipsis-text">{{ lang('NOTIFY_WEB_PUSH_ENABLE') ~ lang('COLON') }}</span>
+				<button id="subscribe_webpush" name="subscribe_webpush" class="notification-subscribe_toggle">{{ Icon('font', 'toggle-off', lang('NOTIFY_WEB_PUSH_SUBSCRIBE'), false, '', {'style' : 'color:#9e9e9e;'}) }}</button>
+				<button id="unsubscribe_webpush" name="unsubscribe_webpush" class="notification-subscribe_toggle hidden">{{ Icon('font', 'toggle-on', lang('NOTIFY_WEB_PUSH_SUBSCRIBED'), false, '', {'style' : 'color:#0059b3;'}) }}</button>
+			</div>
+		{% endif %}
 		{% EVENT notification_dropdown_footer_after %}
 	</div>
 </div>

phpBB/styles/prosilver/template/overall_header.html

@@ -63,6 +63,10 @@
 
 <!-- EVENT overall_header_stylesheets_after -->
 
+{% if NOTIFICATIONS_WEBPUSH_ENABLE %}
+	{% include('ucp_notifications_webpush.html') %}
+{% endif %}
+
 </head>
 <body id="phpbb" class="nojs notouch section-{SCRIPT_NAME} {S_CONTENT_DIRECTION} {BODY_CLASS}">
 

phpBB/styles/prosilver/template/ucp_notifications_options.html

@@ -1,9 +1,5 @@
 {% include('ucp_header.html') %}
 
-{% if NOTIFICATIONS_WEBPUSH_ENABLE %}
-	{% include('ucp_notifications_webpush.html') %}
-{% endif %}
-
 <form id="ucp" method="post" action="{{ S_UCP_ACTION }}"{{ S_FORM_ENCTYPE }}>
 
 	<h2 class="cp-title">{{ TITLE }}</h2>

phpBB/styles/prosilver/theme/buttons.css

@@ -220,3 +220,9 @@ button::-moz-focus-inner {
 .avatar-cropper-buttons > .button-group {
 	margin: 4px;
 }
+
+/* Notification buttons
+--------------------------------------------- */
+.notification-subscribe_toggle:disabled {
+	opacity: 0.7;
+}

phpBB/styles/prosilver/theme/colours.css

@@ -1179,3 +1179,7 @@ input.disabled {
 	background-color: #d41142;
 	color: #ffffff;
 }
+
+.notification-subscribe_toggle {
+	color: #47536b;
+}

phpBB/styles/prosilver/theme/common.css

@@ -1340,6 +1340,15 @@ ul.linklist:after,
 	display: block;
 }
 
+.dropdown-extended .notification-dropdown-footer {
+	white-space: nowrap;
+	border-top: solid 1px #b9b9b9;
+	display: flex;
+	flex-wrap: nowrap;
+	justify-content: space-between;
+	padding: 5px 10px;
+}
+
 .notification-avatar,
 .notification-menu .notification-list .notification-item .avatar,
 .notification-menu .notification-list .notification-item .gravatar {

tests/functional/notification_webpush_test.php

@@ -0,0 +1,127 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+/**
+* @group functional
+*/
+class phpbb_functional_notification_webpush_test extends phpbb_functional_test_case
+{
+	public function test_acp_module()
+	{
+		$this->login();
+		$this->admin_login();
+
+		$this->add_lang(['acp/board', 'acp/common']);
+
+		$crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=webpush&sid=' . $this->sid);
+
+		$this->assertContainsLang('ACP_WEBPUSH_SETTINGS', $crawler->filter('div.main > h1')->text());
+		$this->assertContainsLang('ACP_WEBPUSH_SETTINGS_EXPLAIN', $crawler->filter('div.main > p')->text());
+		$this->assertContainsLang('WEBPUSH_GENERATE_VAPID_KEYS', $crawler->filter('input[type="button"]')->attr('value'));
+
+		$form_data = [
+			'config[webpush_enable]'	=> 1,
+			'config[webpush_vapid_public]'	=> 'BDnYSJHVZBxq834LqDGr893IfazEez7q-jYH2QBNlT0ji2C9UwGosiqz8Dp_ZN23lqAngBZyRjXVWF4ZLA8X2zI',
+			'config[webpush_vapid_private]'	=> 'IE5OYlmfWsMbBU1lzvr0bxrxVAXIteSkAnwGlZIhmRk',
+			'config[webpush_method_default_enable]'	=> 1,
+			'config[webpush_dropdown_subscribe]'	=> 1,
+		];
+		$form = $crawler->selectButton('submit')->form($form_data);
+		$crawler = self::submit($form);
+		$this->assertStringContainsString($this->lang('CONFIG_UPDATED'), $crawler->filter('.successbox')->text());
+
+		$crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=webpush&sid=' . $this->sid);
+
+		foreach ($form_data as $config_name => $config_value)
+		{
+			$config_value = ($config_name === 'config[webpush_vapid_private]') ? '********' : $config_value;
+			$this->assertEquals($config_value, $crawler->filter('input[name="' . $config_name . '"]')->attr('value'));
+		}
+	}
+
+	public function test_ucp_module()
+	{
+		$this->login();
+		$this->admin_login();
+
+		$this->add_lang('ucp');
+
+		$crawler = self::request('GET', 'ucp.php?i=ucp_notifications&mode=notification_options');
+
+		$this->assertContainsLang('NOTIFY_WEBPUSH_ENABLE', $crawler->filter('label[for="subscribe_webpush"]')->text());
+		$this->assertContainsLang('NOTIFICATION_METHOD_WEBPUSH', $crawler->filter('th.mark')->eq(2)->text());
+
+		// Assert checkbox is checked
+		$wp_list = $crawler->filter('.table1');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.bookmark_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.mention_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.post_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.quote_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.topic_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.forum_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.group_request_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.pm_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.report_pm_closed_notification.method.webpush');
+		$this->assert_checkbox_is_checked($wp_list, 'notification.type.report_post_closed_notification.method.webpush');
+
+		$this->set_acp_option('webpush_method_default_enable', 0);
+
+		$crawler = self::request('GET', 'ucp.php?i=ucp_notifications&mode=notification_options');
+
+		// Assert checkbox is unchecked
+		$wp_list = $crawler->filter('.table1');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.bookmark_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.mention_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.post_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.quote_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.topic_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.forum_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.group_request_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.pm_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.report_pm_closed_notification.method.webpush');
+		$this->assert_checkbox_is_unchecked($wp_list, 'notification.type.report_post_closed_notification.method.webpush');
+	}
+
+	public function test_dropdown_subscribe_button()
+	{
+		$this->login();
+		$this->admin_login();
+
+		// Assert subscribe dropdown is present
+		$crawler = self::request('GET', 'index.php');
+		$this->assertCount(1, $crawler->filter('.notification-dropdown-footer'));
+		$this->assertContainsLang('NOTIFY_WEB_PUSH_SUBSCRIBE', $crawler->filter('.notification-dropdown-footer #subscribe_webpush')->text());
+		$this->assertContainsLang('NOTIFY_WEB_PUSH_SUBSCRIBED', $crawler->filter('.notification-dropdown-footer #unsubscribe_webpush')->text());
+
+		// Assert subscribe button is not displayed in UCP when dropdown subscribe is present
+		$crawler = self::request('GET', 'ucp.php?i=ucp_notifications&mode=notification_options');
+		$this->assertCount(0, $crawler->filter('.notification-dropdown-footer'));
+
+		$this->set_acp_option('webpush_dropdown_subscribe', 0);
+
+		// Assert subscribe dropdown is not present by default
+		$crawler = self::request('GET', 'index.php');
+		$this->assertCount(0, $crawler->filter('.notification-dropdown-footer'));
+	}
+
+	protected function set_acp_option($option, $value)
+	{
+		$crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=webpush&sid=' . $this->sid);
+		$form = $crawler->selectButton('Submit')->form();
+		$values = $form->getValues();
+		$values["config[{$option}]"] = $value;
+		$form->setValues($values);
+		$crawler = self::submit($form);
+		$this->assertEquals(1, $crawler->filter('.successbox')->count());
+	}
+}

tests/notification/notification_method_webpush_test.php

@@ -653,6 +653,47 @@ class notification_method_webpush_test extends phpbb_tests_notification_base
 		$this->assertCount(0, $cur_notifications, 'Assert that no notifications have been pruned');
 	}
 
+	public function data_set_endpoint_padding(): array
+	{
+		return [
+			[
+				'foo.mozilla.com',
+				webpush::MOZILLA_FALLBACK_PADDING
+			],
+			[
+				'foo.mozaws.net',
+				webpush::MOZILLA_FALLBACK_PADDING
+			],
+			[
+				'foo.android.googleapis.com',
+				\Minishlink\WebPush\Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH,
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider data_set_endpoint_padding
+	 */
+	public function test_set_endpoint_padding($endpoint, $expected_padding): void
+	{
+		$web_push_reflection = new \ReflectionMethod($this->notification_method_webpush, 'set_endpoint_padding');
+		$web_push_reflection->setAccessible(true);
+
+		$auth = [
+			'VAPID' => [
+				'subject' => generate_board_url(),
+				'publicKey' => $this->config['webpush_vapid_public'],
+				'privateKey' => $this->config['webpush_vapid_private'],
+			],
+		];
+
+		$web_push = new \Minishlink\WebPush\WebPush($auth);
+
+		$this->assertEquals(\Minishlink\WebPush\Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH, $web_push->getAutomaticPadding());
+		$web_push_reflection->invoke($this->notification_method_webpush, $web_push, $endpoint);
+		$this->assertEquals($expected_padding, $web_push->getAutomaticPadding());
+	}
+
 	protected function create_subscription_for_user($user_id, bool $invalidate_endpoint = false): array
 	{
 		$client = new \GuzzleHttp\Client();