Skip to content

Fix: trim whitespace from slot ID exclusions to prevent space-padded …#205

Open
trrill wants to merge 3 commits into10up:developfrom
trrill:bug/excluded_slot_ids_whitespace
Open

Fix: trim whitespace from slot ID exclusions to prevent space-padded …#205
trrill wants to merge 3 commits into10up:developfrom
trrill:bug/excluded_slot_ids_whitespace

Conversation

@trrill
Copy link
Copy Markdown

@trrill trrill commented Apr 9, 2026

…entries from failing to match

Description of the Change

When a comma-separated list of slot IDs is entered in the plugin settings with spaces after commas (e.g. ad--youtube1x1, ad-youtube1x1), the whitespace is preserved when building the $slot_ids_to_exclude_assoc array. This causes the serialized JavaScript object to contain space-padded keys (e.g. " ad-youtube1x1") that will never match a real slot element ID, silently breaking the exclusion for those slots.

The fix adds trim() to each slot ID as it's added to the associative array, consistent with how sizes_to_exclude already handles whitespace via array_map/trim().

How to test the Change

  1. In the plugin settings, add slot IDs to the exclusion list with spaces after commas, e.g. div-gpt-ad-123, div-gpt-ad-456
  2. View page source and inspect the AdRefreshControl JS object
  3. Confirm that slot ID keys in slotIdsToExclude are trimmed and match the actual slot element IDs on the page
  4. Confirm those slots do not refresh

Changelog Entry

Fixed - Trim whitespace from slot ID exclusion list entries to prevent space-padded keys from failing to match slot element IDs

Credits

Props @trrill

Checklist:

@jeffpaul jeffpaul added this to the 1.2.0 milestone Apr 9, 2026
@jeffpaul jeffpaul moved this to Code Review in Open Source Practice Apr 9, 2026
Copy link
Copy Markdown
Author

@trrill trrill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added missing space before closing parentheses to appease the linting gods.

…es when we render in the input, supporting any legacy options that have extra spaces and new options that won't
Copy link
Copy Markdown
Collaborator

@dkotter dkotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @trrill, works as expected. In thinking about it a bit more though, seems ideal that we shouldn't be storing data with extra spaces in the first place. I've pushed a commit that trims that extra space prior to save and then when rendering in the settings input, trim extra spaces (for backwards-compat with previously saved values) and add an extra space back with the comma: c5b4ed6

Your change is still needed to support anyone with previously saved data but then going forward, data will no longer have that extra space.

Let me know if that all makes sense to you though.

@github-project-automation github-project-automation bot moved this from Code Review to QA Testing in Open Source Practice Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: QA Testing

Development

Successfully merging this pull request may close these issues.

3 participants