Fix: GDPR banner duplicate controls & WPML/Polylang i18n#147
Merged
parhumm merged 2 commits intodevelopmentfrom Mar 9, 2026
Merged
Fix: GDPR banner duplicate controls & WPML/Polylang i18n#147parhumm merged 2 commits intodevelopmentfrom
parhumm merged 2 commits intodevelopmentfrom
Conversation
#145) - Strip anchor-only/empty-href links from custom banner message to prevent duplicate Accept/Deny controls alongside rendered buttons. Real URL links (e.g. privacy policy) are preserved. - Register custom GDPR banner strings (message, accept/decline labels) with WPML (wpml_register_single_string) and Polylang (pll_register_string) on settings save, and apply translations at render time via wpml_translate_single_string with native Polylang fallback. - Update banner message field description to clarify link requirements.
📝 WalkthroughWalkthroughAdmin settings now register custom GDPR banner strings with WPML/Polylang after save; the GDPR service translates banner text and button labels at render time and strips anchor-only links from custom messages to avoid duplicate interactive controls. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as "Admin UI"
participant Settings as "Settings Save Handler"
participant WPML as "WPML API"
participant PLL as "Polylang API"
participant Frontend as "GDPRService (render)"
rect rgba(200,200,255,0.5)
Admin->>Settings: Save consent banner & button texts
Settings->>WPML: do_action('wpml_register_single_string', ... ) (if available)
Settings->>PLL: pll_register_string(...) (if function exists)
end
rect rgba(200,255,200,0.5)
Frontend->>WPML: request translation via translateString (if available)
Frontend->>PLL: request translation via translateString (if available)
Frontend->>Frontend: strip anchor-only / empty-href links from message
Frontend->>Frontend: render translated message and buttons
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
Human QA Checklist#144 — Duplicate Controls Fix
#145 — WPML/Polylang i18nWithout translation plugins
With WPML
With Polylang
Regression
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Services/GDPRService.php`:
- Around line 210-212: The preg_replace call that strips anchor-only links in
GDPRService.php currently only matches empty href and bare hash but misses named
anchors; update the regex used in the preg_replace on $message so the href value
matcher accepts either an empty string or a value that begins with a '#'
(including named anchors like "#accept" or "#deny") while still preserving real
URLs — modify the pattern passed to preg_replace in the GDPRService:: (or
surrounding) method so the href value part is an optional group that matches ""
or "#" followed by optional word characters, then keep the replacement as '$1'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e07e0af-d0d7-4c0d-be47-fd92cbc42e8a
📒 Files selected for processing (2)
admin/config/index.phpsrc/Services/GDPRService.php
The previous preg_replace pattern only handled href="#" and href="" but missed named anchors (href="#accept", href="#deny") and whitespace variants (href=" # "). Replace with preg_replace_callback that whitelists real URLs (http(s)://, /path) and strips everything else, preserving inner text content. Tested against 15 edge cases covering all known variants.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Services/GDPRService.php (1)
155-169: Polylang fallback may silently fail to translate.The
pll__()function looks up translations by the original string value, not by name. If Polylang's WPML compatibility layer is disabled, thewpml_translate_single_stringfilter returns the original value unchanged, thenpll__($value)is called—but this only works if$valueexactly matches what was registered. If the stored setting differs even slightly from the registered string (whitespace, encoding), the lookup fails silently.Consider passing the name to Polylang's internal lookup or documenting that the WPML compatibility API must be enabled in Polylang for translations to work reliably.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Services/GDPRService.php` around lines 155 - 169, The Polylang fallback can miss translations when the stored setting doesn't exactly match the original string; update translateString to, after the wpml_translate_single_string filter returns the original, prefer Polylang's name-based lookup if available (call pll_translate_string or any Polylang API that accepts the registration name), and only then fall back to pll__($value); keep the existing checks for function_exists('pll__') and add a short doc comment noting that reliable translations require either Polylang's name-based lookup or Polylang's WPML compatibility API to be enabled. Ensure references to translateString, wpml_translate_single_string, pll_translate_string (if present) and pll__ are used so reviewers can locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Services/GDPRService.php`:
- Around line 210-223: The regex used in the preg_replace_callback that
processes $message (the anonymous function capturing $href and using preg_match)
currently only matches URLs starting with "http(s)://" or a single "/" so
protocol-relative URLs like "//example.com" are stripped despite the comment;
update the preg_match pattern to accept protocol-relative URLs (for example
change '#^(https?://|/)#i' to a pattern that includes '//' such as
'#^((https?://)|//|/)#i' or '#^((https?:)?//|/)#i') so links beginning with "//"
are preserved, and keep the surrounding logic in the anonymous function
unchanged.
---
Nitpick comments:
In `@src/Services/GDPRService.php`:
- Around line 155-169: The Polylang fallback can miss translations when the
stored setting doesn't exactly match the original string; update translateString
to, after the wpml_translate_single_string filter returns the original, prefer
Polylang's name-based lookup if available (call pll_translate_string or any
Polylang API that accepts the registration name), and only then fall back to
pll__($value); keep the existing checks for function_exists('pll__') and add a
short doc comment noting that reliable translations require either Polylang's
name-based lookup or Polylang's WPML compatibility API to be enabled. Ensure
references to translateString, wpml_translate_single_string,
pll_translate_string (if present) and pll__ are used so reviewers can locate the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a55b5e3-8615-4e67-b34b-a91b84459244
📒 Files selected for processing (1)
src/Services/GDPRService.php
Comment on lines
+210
to
+223
| // Strip links that don't point to real URLs (legacy accept/deny controls) | ||
| // Keeps: https://..., http://..., /path, //protocol-relative | ||
| // Strips: #, #fragment, empty, whitespace-only — preserves inner text | ||
| $message = preg_replace_callback( | ||
| '/<a\s[^>]*href\s*=\s*["\']([^"\']*)["\'][^>]*>(.*?)<\/a>/is', | ||
| function ($matches) { | ||
| $href = trim($matches[1]); | ||
| if (preg_match('#^(https?://|/)#i', $href)) { | ||
| return $matches[0]; // Real URL — keep the link | ||
| } | ||
| return $matches[2]; // Not a real URL — keep text, strip tag | ||
| }, | ||
| $message | ||
| ); |
There was a problem hiding this comment.
Protocol-relative URLs are not preserved despite the comment.
The comment on line 211 states //protocol-relative URLs are kept, but the regex ^(https?://|/) requires exactly one leading slash, so //example.com would be stripped. This is likely a minor edge case since protocol-relative URLs are deprecated practice, but the comment is misleading.
🔧 Option to fix the pattern if protocol-relative URLs matter
- if (preg_match('#^(https?://|/)#i', $href)) {
+ if (preg_match('#^(https?://|//)#i', $href)) {Or update the comment to remove the protocol-relative mention if intentionally unsupported.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Strip links that don't point to real URLs (legacy accept/deny controls) | |
| // Keeps: https://..., http://..., /path, //protocol-relative | |
| // Strips: #, #fragment, empty, whitespace-only — preserves inner text | |
| $message = preg_replace_callback( | |
| '/<a\s[^>]*href\s*=\s*["\']([^"\']*)["\'][^>]*>(.*?)<\/a>/is', | |
| function ($matches) { | |
| $href = trim($matches[1]); | |
| if (preg_match('#^(https?://|/)#i', $href)) { | |
| return $matches[0]; // Real URL — keep the link | |
| } | |
| return $matches[2]; // Not a real URL — keep text, strip tag | |
| }, | |
| $message | |
| ); | |
| // Strip links that don't point to real URLs (legacy accept/deny controls) | |
| // Keeps: https://..., http://..., /path, //protocol-relative | |
| // Strips: #, `#fragment`, empty, whitespace-only — preserves inner text | |
| $message = preg_replace_callback( | |
| '/<a\s[^>]*href\s*=\s*["\']([^"\']*)["\'][^>]*>(.*?)<\/a>/is', | |
| function ($matches) { | |
| $href = trim($matches[1]); | |
| if (preg_match('#^(https?://|//)#i', $href)) { | |
| return $matches[0]; // Real URL — keep the link | |
| } | |
| return $matches[2]; // Not a real URL — keep text, strip tag | |
| }, | |
| $message | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Services/GDPRService.php` around lines 210 - 223, The regex used in the
preg_replace_callback that processes $message (the anonymous function capturing
$href and using preg_match) currently only matches URLs starting with
"http(s)://" or a single "/" so protocol-relative URLs like "//example.com" are
stripped despite the comment; update the preg_match pattern to accept
protocol-relative URLs (for example change '#^(https?://|/)#i' to a pattern that
includes '//' such as '#^((https?://)|//|/)#i' or '#^((https?:)?//|/)#i') so
links beginning with "//" are preserved, and keep the surrounding logic in the
anonymous function unchanged.
This was referenced Mar 9, 2026
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
<a>links from custom banner message to prevent duplicate Accept/Deny controls alongside the rendered buttons. Real URL links (e.g. privacy policy pages) are preserved. Updates field description to clarify link requirements.opt_out_message,gdpr_accept_button_text,gdpr_decline_button_text) with WPML (wpml_register_single_string) and Polylang (pll_register_string) on settings save. Applies translations at render time viawpml_translate_single_stringwith native Polylangpll__()fallback. Zero impact when no translation plugin is active.Closes #144
Closes #145
Test plan
opt_out_messagetoClick <a href="#">Accept</a> or <a href="#">Deny</a>— verify anchor-only links are stripped, text preserved, only<button>controls visibleopt_out_messagetoRead our <a href="https://example.com/privacy">Privacy Policy</a>— verify real URL link is preserved and clickable<p>,<br>,<b>,<i>,<strong>,<em>tags still render in banner messagewp-slimstat→ verify translated values render on language switch__()string; empty button text falls back to__('Accept')/__('Deny')🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation