Use browser local storage instead of cookies for closed popups

Author: iMattPro

config/location.yml

@@ -106,10 +106,6 @@ services:
     phpbb.ads.location.type.pop_up:
         class: phpbb\ads\location\type\pop_up
         parent: phpbb.ads.location.type.base
-        arguments:
-            - '@request'
-            - '@config'
-            - '@template'
         tags:
             - { name: phpbb.ads.location.type }
 

location/type/pop_up.php

@@ -12,31 +12,6 @@
 
 class pop_up extends base
 {
-	/** @var \phpbb\request\request */
-	protected $request;
-	/** @var \phpbb\config\config */
-	protected $config;
-	/** @var \phpbb\template\template */
-	protected $template;
-
-	/**
-	 * pop_up location constructor.
-	 *
-	 * @param \phpbb\user              $user     User object
-	 * @param \phpbb\language\language $language Language object
-	 * @param \phpbb\request\request   $request  Request object
-	 * @param \phpbb\config\config     $config   Config object
-	 * @param \phpbb\template\template $template Template object
-	 */
-	public function __construct(\phpbb\user $user, \phpbb\language\language $language, \phpbb\request\request $request, \phpbb\config\config $config, \phpbb\template\template $template)
-	{
-		parent::__construct($user, $language);
-
-		$this->request = $request;
-		$this->config = $config;
-		$this->template = $template;
-	}
-
 	/**
 	 * {@inheritDoc}
 	 */
@@ -52,22 +27,4 @@ public function get_category()
 	{
 		return self::CAT_INTERACTIVE;
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function will_display()
-	{
-		if ($this->request->is_set($this->config['cookie_name'] . '_pop_up', \phpbb\request\request_interface::COOKIE))
-		{
-			return false;
-		}
-
-		$this->template->assign_vars(array(
-			'POP_UP_COOKIE_NAME'	=> $this->config['cookie_name'] . '_pop_up',
-			'POP_UP_COOKIE_EXPIRES'	=> gmdate('D, d M Y H:i:s T', strtotime('+1 day')),
-			'POP_UP_COOKIE_PATH'	=> $this->config['cookie_path'],
-		));
-		return true;
-	}
 }

styles/all/template/includes/phpbb_ads_pop_up.html

@@ -4,8 +4,10 @@
 		'use strict';
 
 		$(window).on('load', function() {
-			document.cookie = '{{ POP_UP_COOKIE_NAME }}=true; expires={{ POP_UP_COOKIE_EXPIRES }}; path={{ POP_UP_COOKIE_PATH }}';
-			phpbb.alert('{{ lang('ADVERTISEMENT')|upper|e('js') }}', `{{ ad.renderAds(AD_POP_UP, AD_POP_UP_ID, AD_POP_UP_CENTER, 'margin: 0;') }}`);
+			if (!localStorage.getItem('phpbb_ads_pop_up')) {
+				localStorage.setItem('phpbb_ads_pop_up', 'true');
+				phpbb.alert('{{ lang('ADVERTISEMENT')|upper|e('js') }}', `{{ ad.renderAds(AD_POP_UP, AD_POP_UP_ID, AD_POP_UP_CENTER, 'margin: 0;') }}`);
+			}
 		});
 	})(jQuery, window, document, phpbb);
 </script>

tests/location/get_all_location_ids_test.php

@@ -20,7 +20,7 @@ class get_all_location_ids_test extends location_base
 	public function get_all_location_ids_data()
 	{
 		return array(
-			array('index', '', false, array(
+			array('index', '', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
@@ -31,7 +31,7 @@ public function get_all_location_ids_data()
 				'slide_up',
 				'scripts',
 			)),
-			array('viewtopic', '', false, array(
+			array('viewtopic', '', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
@@ -48,7 +48,7 @@ public function get_all_location_ids_data()
 				'slide_up',
 				'scripts',
 			)),
-			array('memberlist', 'viewprofile', false, array(
+			array('memberlist', 'viewprofile', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
@@ -61,13 +61,14 @@ public function get_all_location_ids_data()
 				'slide_up',
 				'scripts',
 			)),
-			array('index', '', true, array(
+			array('index', '', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
 				'above_footer',
 				'after_footer_navbar',
 				'below_footer',
+				'pop_up',
 				'slide_up',
 				'scripts',
 			)),
@@ -79,16 +80,11 @@ public function get_all_location_ids_data()
 	 *
 	 * @dataProvider get_all_location_ids_data
 	 */
-	public function test_get_all_location_ids($page_name, $query_string, $cookie, $expected)
+	public function test_get_all_location_ids($page_name, $query_string, $expected)
 	{
 		$this->user->page['page_name'] = $page_name;
 		$this->user->page['query_string'] = $query_string;
 
-		$this->request
-			->method('is_set')
-			->with('_pop_up')
-			->willReturn($cookie);
-
 		$manager = $this->get_manager();
 
 		$location_ids = $manager->get_all_location_ids();

tests/location/location_base.php

@@ -21,9 +21,6 @@ class location_base extends \phpbb_test_case
 	/** @var \phpbb\language\language */
 	protected $language;
 
-	/** @var \phpbb\request\request|\PHPUnit\Framework\MockObject\MockObject */
-	protected $request;
-
 	protected static function setup_extensions()
 	{
 		return array('phpbb/ads');
@@ -41,13 +38,6 @@ protected function setUp(): void
 		$lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx);
 		$this->language = new \phpbb\language\language($lang_loader);
 		$this->user = new \phpbb\user($this->language, '\phpbb\datetime');
-		$this->request = $this->getMockBuilder('\phpbb\request\request')
-			->disableOriginalConstructor()
-			->getMock();
-		$config = new \phpbb\config\config(array());
-		$template = $this->getMockBuilder('\phpbb\template\template')
-			->disableOriginalConstructor()
-			->getMock();
 		// Location types
 		$locations = array(
 			'above_footer',
@@ -72,14 +62,7 @@ protected function setUp(): void
 		foreach ($locations as $type)
 		{
 			$class = "\\phpbb\\ads\\location\\type\\$type";
-			if ($type === 'pop_up')
-			{
-				$location_types['phpbb.ads.location.type.' . $type] = new $class($this->user, $this->language, $this->request, $config, $template);
-			}
-			else
-			{
-				$location_types['phpbb.ads.location.type.' . $type] = new $class($this->user, $this->language);
-			}
+			$location_types['phpbb.ads.location.type.' . $type] = new $class($this->user, $this->language);
 		}
 
 		$this->template_locations = $location_types;