From d6a941d7fbf460bcb0852b71d3541f0583359933 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Sun, 14 Dec 2025 16:59:52 +0000 Subject: [PATCH] fix: prevent empty widget wrapper output when no ad codes found Resolves issue #72 where the ACM_Ad_Zones widget would output empty wrapper HTML (before_widget/after_widget) even when no valid ad codes were found for the specified tag. This created unnecessary empty markup in the page output. The widget now uses output buffering to capture the ad content first, checks if any content was generated, and only outputs the wrapper HTML if there's actual ad content to display. A new filter 'acm_display_empty_widget' has been added to allow themes to override this behaviour if they need the wrapper HTML to display even when empty (defaults to false). Changes: - Modified ACM_Ad_Zones::widget() to buffer ad output and conditionally render wrapper - Added 'acm_display_empty_widget' filter with ad zone, args, and instance parameters - Excluded short prefix warning from PHPCS for established 'acm' prefix - Added 7 integration tests covering empty output behaviour and filter functionality - Added unit tests verifying get_acm_tag() returns empty string when no ad codes found Co-Authored-By: Claude Opus 4.5 --- .phpcs.xml.dist | 2 + src/class-acm-widget.php | 24 ++- tests/Integration/WidgetTest.php | 297 +++++++++++++++++++++++++++++++ tests/Unit/AdCodeManagerTest.php | 100 ++++++++--- 4 files changed, 402 insertions(+), 21 deletions(-) create mode 100644 tests/Integration/WidgetTest.php diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 02b110c..fabf828 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -53,6 +53,8 @@ + + diff --git a/src/class-acm-widget.php b/src/class-acm-widget.php index b3e5d20..ec5724e 100644 --- a/src/class-acm-widget.php +++ b/src/class-acm-widget.php @@ -60,13 +60,35 @@ function update( $new_instance, $old_instance ) { // Display the widget function widget( $args, $instance ) { + // Capture the ad content to check if we have anything to display. + // This fixes issue #72: don't output empty widget wrapper when no ad code found. + ob_start(); + do_action( 'acm_tag', $instance['ad_zone'] ); + $ad_content = ob_get_clean(); + + /** + * Filters whether to display the widget when no ad content is found. + * + * @since 0.8.0 + * + * @param bool $display Whether to display the widget. Default false. + * @param string $ad_zone The ad zone ID. + * @param array $args Widget display arguments. + * @param array $instance Widget instance settings. + */ + if ( empty( $ad_content ) && ! apply_filters( 'acm_display_empty_widget', false, $instance['ad_zone'], $args, $instance ) ) { + return; + } + echo $args['before_widget']; $title = apply_filters( 'widget_title', $instance['title'] ); if ( ! empty( $title ) ) { echo $args['before_title'] . esc_html( $title ) . $args['after_title']; } - do_action( 'acm_tag', $instance['ad_zone'] ); + + // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Ad content is escaped during token replacement in get_acm_tag(). + echo $ad_content; echo $args['after_widget']; } } diff --git a/tests/Integration/WidgetTest.php b/tests/Integration/WidgetTest.php new file mode 100644 index 0000000..3c3ac05 --- /dev/null +++ b/tests/Integration/WidgetTest.php @@ -0,0 +1,297 @@ +widget = new ACM_Ad_Zones(); + } + + /** + * Test widget outputs nothing when no ad code is found. + * + * This is the main fix for issue #72 - the widget should not output + * empty wrapper HTML when there's no ad content to display. + * + * @covers ACM_Ad_Zones::widget + */ + public function test_widget_outputs_nothing_when_no_ad_code_found(): void { + $args = array( + 'before_widget' => '
', + 'after_widget' => '
', + 'before_title' => '

', + 'after_title' => '

', + ); + $instance = array( + 'title' => 'Test Ad', + 'ad_zone' => 'nonexistent_zone', + ); + + ob_start(); + $this->widget->widget( $args, $instance ); + $output = ob_get_clean(); + + $this->assertEmpty( $output, 'Widget should output nothing when no ad code is found.' ); + } + + /** + * Test widget outputs nothing for empty ad zone. + * + * @covers ACM_Ad_Zones::widget + */ + public function test_widget_outputs_nothing_for_empty_ad_zone(): void { + $args = array( + 'before_widget' => '
', + 'after_widget' => '
', + 'before_title' => '

', + 'after_title' => '

', + ); + $instance = array( + 'title' => '', + 'ad_zone' => '', + ); + + ob_start(); + $this->widget->widget( $args, $instance ); + $output = ob_get_clean(); + + $this->assertEmpty( $output, 'Widget should output nothing for empty ad zone.' ); + } + + /** + * Test widget outputs wrapper when filter returns true for empty content. + * + * The acm_display_empty_widget filter allows themes to force display + * of the widget wrapper even when no ad content is found. + * + * @covers ACM_Ad_Zones::widget + */ + public function test_widget_outputs_wrapper_when_filter_returns_true(): void { + add_filter( 'acm_display_empty_widget', '__return_true' ); + + $args = array( + 'before_widget' => '
', + 'after_widget' => '
', + 'before_title' => '

', + 'after_title' => '

', + ); + $instance = array( + 'title' => '', + 'ad_zone' => 'nonexistent_zone', + ); + + ob_start(); + $this->widget->widget( $args, $instance ); + $output = ob_get_clean(); + + remove_filter( 'acm_display_empty_widget', '__return_true' ); + + $this->assertStringContainsString( '
', $output, 'Widget should output wrapper when filter returns true.' ); + $this->assertStringContainsString( '
', $output, 'Widget should output closing wrapper when filter returns true.' ); + } + + /** + * Test widget outputs title when filter allows empty content. + * + * @covers ACM_Ad_Zones::widget + */ + public function test_widget_outputs_title_when_filter_allows_empty_content(): void { + add_filter( 'acm_display_empty_widget', '__return_true' ); + + $args = array( + 'before_widget' => '
', + 'after_widget' => '
', + 'before_title' => '

', + 'after_title' => '

', + ); + $instance = array( + 'title' => 'Advertisement', + 'ad_zone' => 'nonexistent_zone', + ); + + ob_start(); + $this->widget->widget( $args, $instance ); + $output = ob_get_clean(); + + remove_filter( 'acm_display_empty_widget', '__return_true' ); + + $this->assertStringContainsString( '

Advertisement

', $output, 'Widget should output title when filter allows.' ); + } + + /** + * Test acm_display_empty_widget filter receives correct arguments. + * + * @covers ACM_Ad_Zones::widget + */ + public function test_filter_receives_correct_arguments(): void { + $received_args = array(); + + $filter_callback = function ( $display, $ad_zone, $args, $instance ) use ( &$received_args ) { + $received_args = array( + 'display' => $display, + 'ad_zone' => $ad_zone, + 'args' => $args, + 'instance' => $instance, + ); + return false; + }; + + add_filter( 'acm_display_empty_widget', $filter_callback, 10, 4 ); + + $args = array( + 'before_widget' => '
', + 'after_widget' => '
', + 'before_title' => '

', + 'after_title' => '

', + ); + $instance = array( + 'title' => 'Test', + 'ad_zone' => 'test_zone', + ); + + ob_start(); + $this->widget->widget( $args, $instance ); + ob_get_clean(); + + remove_filter( 'acm_display_empty_widget', $filter_callback, 10 ); + + $this->assertFalse( $received_args['display'], 'First argument should be false.' ); + $this->assertSame( 'test_zone', $received_args['ad_zone'], 'Second argument should be the ad zone.' ); + $this->assertSame( $args, $received_args['args'], 'Third argument should be widget args.' ); + $this->assertSame( $instance, $received_args['instance'], 'Fourth argument should be widget instance.' ); + } + + /** + * Test widget with valid ad code outputs content. + * + * @covers ACM_Ad_Zones::widget + */ + public function test_widget_outputs_content_with_valid_ad_code(): void { + // Create a valid ad code. + $ad_code_data = array(); + foreach ( $this->acm->current_provider->ad_code_args as $arg ) { + $ad_code_data[ $arg['key'] ] = 'test_value_' . $arg['key']; + } + $ad_code_data['priority'] = 10; + $ad_code_data['operator'] = 'AND'; + + // Need to set up a tag that's registered. + // First, let's check what tags are available. + $ad_tag_ids = $this->acm->ad_tag_ids; + if ( empty( $ad_tag_ids ) ) { + $this->markTestSkipped( 'No ad tag IDs available for testing.' ); + } + + // Get the first tag with enable_ui_mapping. + $test_tag = null; + foreach ( $ad_tag_ids as $tag ) { + if ( isset( $tag['enable_ui_mapping'] ) && $tag['enable_ui_mapping'] ) { + $test_tag = $tag['tag']; + break; + } + } + + if ( ! $test_tag ) { + $this->markTestSkipped( 'No tags with enable_ui_mapping available for testing.' ); + } + + // Set the tag in ad code data. + $ad_code_data['tag'] = $test_tag; + + // Create the ad code. + $ad_code_id = $this->acm->create_ad_code( $ad_code_data ); + $this->assertIsInt( $ad_code_id, 'Ad code should be created successfully.' ); + + // Register ad codes to make them available. + $this->acm->register_ad_codes( $this->acm->get_ad_codes() ); + + // Enable display of ad codes without conditionals. + add_filter( 'acm_display_ad_codes_without_conditionals', '__return_true' ); + + $args = array( + 'before_widget' => '
', + 'after_widget' => '
', + 'before_title' => '

', + 'after_title' => '

', + ); + $instance = array( + 'title' => 'Ad Widget', + 'ad_zone' => $test_tag, + ); + + ob_start(); + $this->widget->widget( $args, $instance ); + $output = ob_get_clean(); + + remove_filter( 'acm_display_ad_codes_without_conditionals', '__return_true' ); + + // Clean up. + $this->acm->delete_ad_code( $ad_code_id ); + + $this->assertStringContainsString( '
', $output, 'Widget should output wrapper with valid ad code.' ); + $this->assertStringContainsString( '
', $output, 'Widget should output closing wrapper.' ); + } + + /** + * Test that acm_tag action is still fired (backwards compatibility). + * + * @covers ACM_Ad_Zones::widget + */ + public function test_acm_tag_action_is_fired(): void { + $action_fired = false; + $action_tag = null; + + $action_callback = function ( $tag_id ) use ( &$action_fired, &$action_tag ) { + $action_fired = true; + $action_tag = $tag_id; + }; + + add_action( 'acm_tag', $action_callback, 5 ); // Priority 5 to run before the default handler. + + $args = array( + 'before_widget' => '
', + 'after_widget' => '
', + 'before_title' => '

', + 'after_title' => '

', + ); + $instance = array( + 'title' => '', + 'ad_zone' => 'test_action_zone', + ); + + ob_start(); + $this->widget->widget( $args, $instance ); + ob_get_clean(); + + remove_action( 'acm_tag', $action_callback, 5 ); + + $this->assertTrue( $action_fired, 'acm_tag action should be fired for backwards compatibility.' ); + $this->assertSame( 'test_action_zone', $action_tag, 'acm_tag action should receive the correct tag ID.' ); + } +} diff --git a/tests/Unit/AdCodeManagerTest.php b/tests/Unit/AdCodeManagerTest.php index 72827fe..c059873 100644 --- a/tests/Unit/AdCodeManagerTest.php +++ b/tests/Unit/AdCodeManagerTest.php @@ -32,17 +32,17 @@ protected function setUp(): void { parent::setUp(); // Stub WordPress functions. - Functions\stubs( [ '__' => null ] ); + Functions\stubs( array( '__' => null ) ); $this->ad_code_manager = new Ad_Code_Manager(); // Create a mock provider with whitelisted URLs. - $this->ad_code_manager->current_provider = new stdClass(); - $this->ad_code_manager->current_provider->whitelisted_script_urls = [ + $this->ad_code_manager->current_provider = new stdClass(); + $this->ad_code_manager->current_provider->whitelisted_script_urls = array( 'example.com', 'ads.google.com', 'secure.pagead2.googlesyndication.com', - ]; + ); } /** @@ -124,16 +124,16 @@ public function testValidateScriptUrlDeepSubdomain(): void { * Test filter_output_tokens adds URL vars as tokens. */ public function testFilterOutputTokensAddsUrlVars(): void { - $code_to_display = [ - 'url_vars' => [ - 'site_id' => '12345', - 'zone' => 'header', - 'width' => '728', - 'height' => '90', - ], - ]; + $code_to_display = array( + 'url_vars' => array( + 'site_id' => '12345', + 'zone' => 'header', + 'width' => '728', + 'height' => '90', + ), + ); - $output_tokens = $this->ad_code_manager->filter_output_tokens( [], 'test_tag', $code_to_display ); + $output_tokens = $this->ad_code_manager->filter_output_tokens( array(), 'test_tag', $code_to_display ); $this->assertArrayHasKey( '%site_id%', $output_tokens ); $this->assertArrayHasKey( '%zone%', $output_tokens ); @@ -147,8 +147,8 @@ public function testFilterOutputTokensAddsUrlVars(): void { * Test filter_output_tokens returns original tokens when no URL vars. */ public function testFilterOutputTokensReturnsOriginalWhenNoUrlVars(): void { - $code_to_display = []; - $original_tokens = [ '%existing%' => 'value' ]; + $code_to_display = array(); + $original_tokens = array( '%existing%' => 'value' ); $output_tokens = $this->ad_code_manager->filter_output_tokens( $original_tokens, @@ -163,12 +163,12 @@ public function testFilterOutputTokensReturnsOriginalWhenNoUrlVars(): void { * Test filter_output_tokens preserves existing tokens. */ public function testFilterOutputTokensPreservesExistingTokens(): void { - $code_to_display = [ - 'url_vars' => [ + $code_to_display = array( + 'url_vars' => array( 'new_var' => 'new_value', - ], - ]; - $original_tokens = [ '%existing%' => 'value' ]; + ), + ); + $original_tokens = array( '%existing%' => 'value' ); $output_tokens = $this->ad_code_manager->filter_output_tokens( $original_tokens, @@ -179,4 +179,64 @@ public function testFilterOutputTokensPreservesExistingTokens(): void { $this->assertArrayHasKey( '%existing%', $output_tokens ); $this->assertArrayHasKey( '%new_var%', $output_tokens ); } + + /** + * Test get_acm_tag returns empty string when ad rendering is disabled. + * + * @covers Ad_Code_Manager::get_acm_tag + */ + public function testGetAcmTagReturnsEmptyWhenRenderingDisabled(): void { + Functions\when( 'is_preview' )->justReturn( false ); + Functions\when( 'apply_filters' )->alias( + function ( $filter_name, $value ) { + if ( 'acm_disable_ad_rendering' === $filter_name ) { + return true; // Disable rendering. + } + return $value; + } + ); + + $result = $this->ad_code_manager->get_acm_tag( 'test_tag' ); + + $this->assertSame( '', $result ); + } + + /** + * Test get_acm_tag returns empty string when no ad codes registered for tag. + * + * This verifies the fix for issue #72 - no broken HTML output when no ad codes found. + * + * @covers Ad_Code_Manager::get_acm_tag + */ + public function testGetAcmTagReturnsEmptyWhenNoAdCodesRegistered(): void { + Functions\when( 'apply_filters' )->alias( + function ( $filter_name, $value ) { + if ( 'acm_disable_ad_rendering' === $filter_name ) { + return false; // Rendering enabled. + } + return $value; + } + ); + Functions\when( 'is_preview' )->justReturn( false ); + Functions\when( 'wp_cache_get' )->justReturn( false ); + Functions\when( 'wp_cache_add' )->justReturn( true ); + + // Ensure no ad codes are registered for this tag. + $this->assertArrayNotHasKey( 'nonexistent_tag', $this->ad_code_manager->ad_codes ); + + $result = $this->ad_code_manager->get_acm_tag( 'nonexistent_tag' ); + + $this->assertSame( '', $result ); + } + + /** + * Test get_matching_ad_code returns null when tag has no registered ad codes. + * + * @covers Ad_Code_Manager::get_matching_ad_code + */ + public function testGetMatchingAdCodeReturnsNullWhenNoAdCodes(): void { + $result = $this->ad_code_manager->get_matching_ad_code( 'nonexistent_tag' ); + + $this->assertNull( $result ); + } }