fix: prevent empty widget wrapper output when no ad codes found #186
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When the ACM Ad Zones widget was configured for an ad zone without any matching ad codes, the widget would still render its wrapper HTML elements. This resulted in broken output containing unreplaced tokens such as
%width%,%height%, and%tag_id%in the page source. Beyond being visually problematic, this cluttered the DOM with meaningless elements and exposed implementation details that should remain hidden when no ad content exists.The root cause was that
ACM_Ad_Zones::widget()calleddo_action('acm_tag', ...)directly within the widget wrapper output, without first checking whether any ad content would actually be rendered. The action hook would fire, find no matching ad code, and return nothing—but the surrounding wrapper HTML had already been echoed.Solution
This change modifies
ACM_Ad_Zones::widget()to use output buffering to capture the ad content before deciding whether to output the widget wrapper. The method now:acm_tagaction usingob_start()andob_get_clean()acm_display_empty_widgetfilter doesn't override this behaviourA new filter,
acm_display_empty_widget, has been introduced to allow themes or plugins to override this behaviour and force display of empty widget wrappers if desired. The filter receives the ad zone identifier, widget arguments, and widget instance for contextual decision-making.Additional changes include excluding the short prefix warning from PHPCS for the established "acm" prefix, which has been consistently used throughout this plugin's history.
The fix includes comprehensive test coverage: seven integration tests verify widget empty output behaviour across various scenarios (including filter interaction and backwards compatibility), whilst three unit tests confirm that
get_acm_tag()correctly returns empty strings in appropriate circumstances.Fixes #72