-
Notifications
You must be signed in to change notification settings - Fork 32
v4.6.3 #361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
|
PR Summary: Release v4.6.3 — Adds improved WordPress 5.7+ script consent handling (including inline scripts) and fixes banner disablement issues after updates/migrations.
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
Nitpicks 🔍
|
| $enabled = get_option( 'cookiebot-banner-enabled', 'default' ); | ||
| $cbid = Cookiebot_WP::get_cbid(); | ||
|
|
||
| // Set to enabled if it's a fresh install (default) | ||
| if ( $enabled === 'default' ) { | ||
| $enabled = '1'; | ||
| update_option( 'cookiebot-banner-enabled', $enabled ); | ||
| } | ||
|
|
||
| // If banner is disabled but CBID is configured, it was disabled by hosting or plugin update. Re-enable it. | ||
| if ( $enabled === '0' && ! empty( $cbid ) ) { | ||
| update_option( 'cookiebot-banner-enabled', '1' ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CRITICAL_BUG] This change forcefully re-enables the banner when option 'cookiebot-banner-enabled' === '0' and a CBID exists (lines 98-100). Two issues: (1) On multisite installations the code uses update_option() which will not update the network/site option if the setting is stored as a site option — use update_site_option() when is_multisite() to keep behavior consistent with set_to_mode_auto_when_no_cookiebot_id_is_set(). (2) This unconditional re-enable can override an administrator's intentional decision to disable the banner. Instead, scope the auto re-enable to only known-recovery scenarios (for example: during activation/migration flows protected by a transient, or when detecting a recent plugin-version change) and/or log/admin-notice the action so admins are aware. Example fix snippet: if ( $enabled === '0' && ! empty( $cbid ) && get_transient( 'cookiebot_reenable_banner_recovery' ) ) { if ( is_multisite() ) { update_site_option( 'cookiebot-banner-enabled', '1' ); } else { update_option( 'cookiebot-banner-enabled', '1' ); } }
private function set_banner_enabled_by_default() {
$enabled = get_option( 'cookiebot-banner-enabled', 'default' );
$cbid = Cookiebot_WP::get_cbid();
// Set to enabled if it's a fresh install (default)
if ( $enabled === 'default' ) {
$enabled = '1';
update_option( 'cookiebot-banner-enabled', $enabled );
}
// Only attempt automatic recovery when explicitly in a recovery window
if ( $enabled === '0' && ! empty( $cbid ) && get_transient( 'cookiebot_reenable_banner_recovery' ) ) {
if ( is_multisite() ) {
update_site_option( 'cookiebot-banner-enabled', '1' );
} else {
update_option( 'cookiebot-banner-enabled', '1' );
}
Cookiebot_WP::debug_log( 'Banner was automatically re-enabled as part of recovery flow.' );
}
}| public function cookiebot_add_consent_attribute_to_script_tag( $attributes ) { | ||
| // First, check if this script requires consent (registered via add_tag) | ||
| $handle = $this->extract_handle_from_attributes( $attributes ); | ||
|
|
||
| if ( $handle && array_key_exists( $handle, $this->tags ) && ! empty( $this->tags[ $handle ] ) ) { | ||
| $attributes['type'] = 'text/plain'; | ||
| $attributes['data-cookieconsent'] = implode( ',', $this->tags[ $handle ] ); | ||
| return $attributes; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CRITICAL_BUG] In cookiebot_add_consent_attribute_to_script_tag you force attributes['type'] = 'text/plain' for scripts that require consent (lines 116-117). Overriding the type unconditionally can break legitimate script types (e.g. type='module') or other non-JS content. Change the logic to: (1) detect and preserve module scripts (if isset($attributes['type']) && $attributes['type'] === 'module' then skip or handle differently), (2) only override type when no type is set or when it's safe to change, and (3) consider adding a whitelist/opt-in for types you can convert to text/plain. Example: if ( empty( $attributes['type'] ) || $attributes['type'] === 'text/javascript' ) { $attributes['type'] = 'text/plain'; }
public function cookiebot_add_consent_attribute_to_script_tag( $attributes ) {
$handle = $this->extract_handle_from_attributes( $attributes );
if ( $handle && array_key_exists( $handle, $this->tags ) && ! empty( $this->tags[ $handle ] ) ) {
// Preserve module scripts and other explicit non-JS types
if ( empty( $attributes['type'] ) || $attributes['type'] === 'text/javascript' || $attributes['type'] === 'application/javascript' ) {
$attributes['type'] = 'text/plain';
}
$attributes['data-cookieconsent'] = implode( ',', $this->tags[ $handle ] );
return $attributes;
}
if ( isset( $attributes['src'] ) && $this->check_ignore_script( $attributes['src'] ) ) {
$attributes['data-cookieconsent'] = 'ignore';
}
return $attributes;
}| private function extract_handle_from_attributes( $attributes ) { | ||
| if ( ! isset( $attributes['id'] ) ) { | ||
| return null; | ||
| } | ||
|
|
||
| $id = $attributes['id']; | ||
|
|
||
| // Remove the '-js' suffix that WordPress adds | ||
| if ( substr( $id, -3 ) === '-js' ) { | ||
| return substr( $id, 0, -3 ); | ||
| } | ||
|
|
||
| // If no -js suffix, return the ID as-is (some scripts may not follow convention) | ||
| return $id; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[REFACTORING] extract_handle_from_attributes strips only the literal '-js' suffix (lines 223-225) and returns the id as-is for other patterns. Improve robustness by removing '-js' and optional extra/after/before suffixes in one step (same as extract_base_id_from_inline_id suggestion). Example:
private function extract_handle_from_attributes( $attributes ) {
if ( ! isset( $attributes['id'] ) ) {
return null;
}
$id = $attributes['id'];
// Remove the '-js' suffix and optional inline suffixes that WordPress may add
$handle = preg_replace( '/-js(?:-(?:extra|after|before))?$/', '', $id );
return $handle !== '' ? $handle : null;
}|
Reviewed up to commit:2396a4f1bcfcb3bd824d0cc44f729061187b01c6 Additional Suggestionsrc/lib/Cookiebot_Deactivated.php, line:14-15You removed the call that disabled the banner on deactivation (previously disable_banner()). This will keep the banner option intact when the plugin is deactivated. If this was intentional as part of the bugfix (to avoid disabling the banner after updates/migrations), please document the decision. If not intentional, re-check whether deactivation should still clear/disable front-facing behavior or provide an admin-facing toggle to control this behavior. Also ensure uninstall() still cleans up expected options when a full uninstall is requested.// In src/lib/Cookiebot_Deactivated.php
class Cookiebot_Deactivated {
/**
* Plugin deactivation hook.
*
* Note: We intentionally keep the `cookiebot-banner-enabled` option
* unchanged on deactivation so that the banner configuration is
* preserved across plugin updates and hosting migrations.
*
* Full removal of options (including banner state) is handled by
* the uninstall routine instead.
*
* @throws Exception
*/
public function run() {
$this->run_addons_deactivation_hooks();
// Intentionally do not disable the banner here to avoid
// unintentionally turning it off across updates/migrations.
}
/**
* @throws Exception
*/
private function run_addons_deactivation_hooks() {
$cookiebot_addons = Cookiebot_Addons::instance();
$cookiebot_addons->cookiebot_deactivated();
}
}src/lib/script_loader_tag/Script_Loader_Tag.php, line:177-205extract_base_id_from_inline_id currently strips only '-js-(extra|after|before)' suffix (line 179/205). It won't remove a plain '-js' suffix (e.g. 'my-handle-js'), which means mapping inline scripts to their base handle can fail. Use a single regex to remove '-js' and optional '-js-' forms. Example replacement: return preg_replace('/-js(?:-(?:extra|after|before))?$/', '', $inline_script_id); This will improve matching of inline script IDs to registered handles.private function extract_base_id_from_inline_id( $inline_script_id ) {
// Strip -js and optional -js-(extra|after|before) suffix to get the base ID.
return preg_replace( '/-js(?:-(?:extra|after|before))?$/', '', $inline_script_id );
} |
|
CodeAnt AI finished reviewing your PR. |



User description
User description
Improvements
Bug fixes
PR Type
Bug fix, Enhancement
Description
Fixed consent banner disabling after plugin updates or hosting migrations
Enhanced script consent handling for WordPress 5.7+ with inline script support
Improved script tag attribute extraction for proper consent attribute injection
Removed banner disable logic from plugin deactivation process
Diagram Walkthrough
File Walkthrough
cookiebot.php
Version bump to 4.6.3cookiebot.php
Cookiebot_WP.php
Update version constantsrc/lib/Cookiebot_WP.php
Cookiebot_Activated.php
Fix banner disable on updates and migrationssrc/lib/Cookiebot_Activated.php
disables
Cookiebot_Deactivated.php
Remove banner disable on deactivationsrc/lib/Cookiebot_Deactivated.php
disable_banner()from deactivation processScript_Loader_Tag.php
Enhance consent attribute handling for inline scriptssrc/lib/script_loader_tag/Script_Loader_Tag.php
cookiebot_add_consent_attribute_to_script_tag()to handleconsent-required scripts via
add_tagregistrationextract_handle_from_attributes()method to extract scripthandles from tag attributes
cookiebot_add_consent_attribute_to_inline_script_tag()tosupport inline scripts with consent requirements
scripts
external and inline scripts
readme.txt
Update readme with v4.6.3 release notesreadme.txt
CodeAnt-AI Description
Restore banner when CBID present and add consent handling for WordPress 5.7+
What Changed
Impact
✅ Banner restored after plugin updates or hosting migrations✅ Correct consent enforcement for inline scripts on WP 5.7+✅ Banner not removed on plugin deactivation💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.