-
Notifications
You must be signed in to change notification settings - Fork 32
replace hardcoded script tags with core api #360
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
base: master
Are you sure you want to change the base?
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 |
|||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
|
PR Summary: Replace hardcoded <script> tags with WordPress core script APIs and build attribute arrays for safer, standardized output. Changes:
Potential breaking/behavior changes to review:
|
Nitpicks 🔍
|
| 'id' => 'Cookiebot', | ||
| 'src' => 'https://consent.cookiebot.com/uc.js', | ||
| 'data-implementation' => 'wp', | ||
| 'data-cbid' => $cbid, |
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.
Suggestion: Unsanitized assignment to data-cbid: the value from $cbid is assigned directly to attributes array without escaping, which can lead to unsafe output; escape the value with esc_attr() when building attributes. [security]
Severity Level: Critical 🚨
- ❌ Cookiebot frontend attribute may enable XSS.
- ⚠️ Plugin admin-entered cbid values are risk vectors.| 'data-cbid' => $cbid, | |
| 'data-cbid' => esc_attr( $cbid ), |
Steps of Reproduction ✅
1. In a test WordPress environment or simple PHP harness, set $cbid to a value containing
special characters, e.g. $cbid = '" onmouseover="alert(1)'. Save this value as if it were
coming from plugin configuration.
2. Include the template file: src/view/frontend/scripts/cb_frame/cookiebot-js.php. The
attribute array is built at lines 11-17 and includes 'data-cbid' => $cbid (line 16).
3. The template calls wp_print_script_tag() at line 45, which will emit the <script> with
attributes composed from $script_attributes.
4. Inspect the page source / rendered output: if $cbid is not escaped before being passed
into the attributes array, attribute content may be injected verbatim, demonstrating
unescaped attribute output and potential XSS.
Note: This reproduction references the exact assignment (line 16) and the emission point
wp_print_script_tag() (line 45) in the same file.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/view/frontend/scripts/cb_frame/cookiebot-js.php
**Line:** 16:16
**Comment:**
*Security: Unsanitized assignment to `data-cbid`: the value from `$cbid` is assigned directly to attributes array without escaping, which can lead to unsafe output; escape the value with `esc_attr()` when building attributes.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| if ( (bool) get_option( 'cookiebot-gtm' ) !== false && ! empty( get_option( 'cookiebot-data-layer' ) ) ) { | ||
| $script_attributes['data-layer-name'] = get_option( 'cookiebot-data-layer' ); |
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.
Suggestion: Unescaped option value used for data-layer-name: get_option( 'cookiebot-data-layer' ) is assigned directly to attributes array without escaping; fetch the option once, check it, and escape it with esc_attr() before use. [security]
Severity Level: Critical 🚨
- ❌ Admin-set option may inject unsafe frontend attribute.
- ⚠️ Data layer integration could be exploited.| if ( (bool) get_option( 'cookiebot-gtm' ) !== false && ! empty( get_option( 'cookiebot-data-layer' ) ) ) { | |
| $script_attributes['data-layer-name'] = get_option( 'cookiebot-data-layer' ); | |
| $layer = get_option( 'cookiebot-data-layer' ); | |
| if ( (bool) get_option( 'cookiebot-gtm' ) !== false && ! empty( $layer ) ) { | |
| $script_attributes['data-layer-name'] = esc_attr( $layer ); |
Steps of Reproduction ✅
1. As a site admin, set the option 'cookiebot-data-layer' to a payload value via the WP
options UI or direct DB write (e.g. data like '" onclick="alert(1)').
2. Load a frontend page that renders the plugin template
src/view/frontend/scripts/cb_frame/cookiebot-js.php.
3. The conditional at line 31 checks options and (when true) assigns
get_option('cookiebot-data-layer') directly into $script_attributes at line 32.
4. The template emits the script attributes with wp_print_script_tag() at line 45; because
the option value was not escaped before assignment, the output can contain unescaped
content, demonstrating possible XSS via an admin-configured option.
Note: Steps reference concrete code locations: option fetch and assignment (lines 31-32)
and emission at line 45.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/view/frontend/scripts/cb_frame/cookiebot-js.php
**Line:** 31:32
**Comment:**
*Security: Unescaped option value used for `data-layer-name`: `get_option( 'cookiebot-data-layer' )` is assigned directly to attributes array without escaping; fetch the option once, check it, and escape it with `esc_attr()` before use.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.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.
See wp_sanitize_script_attributes() which runs esc_attr() on all passed atribtues
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.
Question:
See wp_sanitize_script_attributes() which runs esc_attr() on all passed atribtues
Answer:
If wp_print_script_tag() (via wp_sanitize_script_attributes()) already escapes all attribute values with esc_attr(), then the “unescaped attribute → XSS” claim is likely a false positive (you’d have double-escaping at worst, not a security fix).
To reduce these kinds of WP-specific false positives in /improve suggestions, add an instruction to verify whether output is already sanitized/escaped by the WordPress API used at render time before flagging a security issue or proposing esc_*() changes.
[pr_code_suggestions]
extra_instructions = """\
- For WordPress/PHP code: before suggesting esc_attr()/esc_html()/esc_url() (or tagging as security/XSS), verify whether the output API already sanitizes/escapes (e.g., wp_print_script_tag() -> wp_sanitize_script_attributes()).
- If escaping is already applied by the sink function, do not recommend redundant escaping; instead, briefly mention why it's already safe.
- Prefer referencing the escaping/sanitization function in the rationale when concluding it's safe.
"""
#### Relevant Sources:
> - https://docs.qodo.ai/qodo-documentation/qodo-merge/tools/improve#extra-instructions-and-best-practices
> - https://docs.qodo.ai/qodo-documentation/qodo-merge/faq/index#-note-"q:-i-received-an-incorrect-or-irrelevant-suggestion-why"
> - https://docs.qodo.ai/qodo-documentation/qodo-merge/usage-guide/additional_configurations#extra-instructions| if ($consent_attribute) { | ||
| $attributes['data-cookieconsent'] = $consent_attribute; |
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.
Suggestion: Undefined variable bug: the code assigns the cookie-consent attribute into $attributes, but the attributes array is named $script_attributes, so $attributes is undefined and the consent attribute is never added; also sanitize the consent value before output to avoid injecting untrusted data into HTML attributes. [null pointer]
Severity Level: Critical 🚨
- ⚠️ data-cookieconsent attribute missing on GTM script.
- ⚠️ PHP notice "Undefined variable: attributes" logged.
- ⚠️ Consent-driven script behavior may be affected.| if ($consent_attribute) { | |
| $attributes['data-cookieconsent'] = $consent_attribute; | |
| if ( $consent_attribute ) { | |
| $script_attributes['data-cookieconsent'] = sanitize_text_field( $consent_attribute ); |
Steps of Reproduction ✅
1. Render the frontend template that includes this script template file:
src/view/frontend/scripts/common/google-tag-manager-js.php. The template content at lines
10-15 sets up $script_attributes and conditionally writes the consent attribute.
2. Ensure the template is executed with a truthy $consent_attribute value (the file's
docblock shows @var bool|string $consent_attribute; the including code must provide this
variable). When that happens, execution reaches lines 13-14 where the code attempts to
assign the consent attribute.
3. At lines 13-14 the code writes to $attributes['data-cookieconsent'] while the
associative array in use is $script_attributes (lines 10-12). Because $attributes is
undefined, PHP emits an "Undefined variable: attributes" notice and the consent value is
not stored on $script_attributes.
4. Later the same file calls wp_print_inline_script_tag( ob_get_clean(),
$script_attributes ) (lines 37-38). Because the consent key was never added to
$script_attributes, the produced inline <script> will be missing the data-cookieconsent
attribute and PHP logs a notice. This is a real execution path entirely inside this
template file (no external callers must be inferred beyond the template including
mechanism).Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/view/frontend/scripts/common/google-tag-manager-js.php
**Line:** 13:14
**Comment:**
*Null Pointer: Undefined variable bug: the code assigns the cookie-consent attribute into `$attributes`, but the attributes array is named `$script_attributes`, so `$attributes` is undefined and the consent attribute is never added; also sanitize the consent value before output to avoid injecting untrusted data into HTML attributes.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.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.
The undefined variable bug has been fixed but sanitization would be out of scope and change the current functionality. Also note that core apis sanitize the value with esc_attr() as mentioned in another comment.
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.
Question:
The undefined variable bug has been fixed but sanitization would be out of scope and change the current functionality. Also note that core apis sanitize the value with esc_attr() as mentioned in another comment.
Answer:
To reduce “sanitize/escape this” recommendations in cases like this (where output is already escaped by a core API such as esc_attr() inside wp_print_inline_script_tag(...), and adding sanitization would be out-of-scope / change behavior), guide the /improve tool with pr_code_suggestions.extra_instructions.
Add to your .pr_agent.toml (repo / wiki / global):
[pr_code_suggestions]
extra_instructions = """\
- Avoid suggesting additional sanitization/escaping for HTML attributes if the value is already escaped by the sink (e.g., WordPress core escapes attributes via esc_attr() / APIs like wp_print_inline_script_tag()).
- Do not propose “security hardening” changes that would alter current behavior or are out-of-scope for the PR (e.g., adding sanitize_text_field() when the variable may be bool|string or when sanitization policy is handled elsewhere).
- Prioritize functional correctness and concrete runtime bugs (e.g., undefined variable / missing attribute) over speculative security suggestions.
"""Note: this won’t eliminate all false-positive security suggestions (LLM limitation), but it typically reduces them and makes the model align better with your project’s escaping/sanitization policy.
Relevant Sources:
- https://docs.qodo.ai/qodo-documentation/qodo-merge/tools/improve#extra-instructions-and-best-practices
- https://docs.qodo.ai/qodo-documentation/qodo-merge/tools/improve#extra-instructions
- https://docs.qodo.ai/qodo-documentation/qodo-merge/faq/index#-note-"q:-i-received-an-incorrect-or-irrelevant-suggestion-why"
- https://docs.qodo.ai/qodo-documentation/qodo-merge/usage-guide/configuration_options#wiki-configuration-file
|
CodeAnt AI finished reviewing your PR. |
| } elseif ( ! empty( $tag_attr ) ) { | ||
| $script_attributes[ $tag_attr ] = true; | ||
| } |
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 line sets $script_attributes[ $tag_attr ] = true; but $tag_attr in the original code was a prebuilt attribute string (e.g. 'type="text/plain" data-cookieconsent="..."'). Using it as an array key will produce an invalid attribute and break output. Restore the original behavior by either: (a) parsing $tag_attr into key/value pairs and merging into $script_attributes, or (b) when $cookie_blocking_mode !== 'auto' output the raw $tag_attr string instead of using wp_print_script_tag. Example actionable fix: parse attributes with preg_match_all('/([\w:-]+)(?:\s*=\s*"([^"]*)")?/', $tag_attr, $m) and set $script_attributes[$m[1][$i]] = $m[2][$i] for each match before calling wp_print_script_tag.
if ( $cookie_blocking_mode === 'auto' ) {
$script_attributes['data-blockingmode'] = 'auto';
} elseif ( ! empty( $tag_attr ) ) {
// Preserve legacy behavior for non-auto mode by parsing composite attributes.
if ( preg_match_all( '/([\w:-]+)(?:\s*=\s*"([^"]*)")?/', $tag_attr, $matches, PREG_SET_ORDER ) ) {
foreach ( $matches as $match ) {
$attr_name = $match[1];
$attr_value = isset( $match[2] ) ? $match[2] : true;
$script_attributes[ $attr_name ] = $attr_value;
}
}
}
wp_print_script_tag( $script_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.
This is wrong. The tag_attr comes from the database option. which can be empty, defer or async.
wp_print_script_tag() handles booleans correctly: https://github.com/WordPress/wordpress-develop/blob/6.9/src/wp-includes/script-loader.php#L2884-L2888
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.
Question:
This is wrong. The tag_attr comes from the database option. which can be empty, defer or async. wp_print_script_tag() handles booleans correctly: https://github.com/WordPress/wordpress-develop/blob/6.9/src/wp-includes/script-loader.php#L2884-L2888
Answer:
This looks like a classic “wrong assumption about variable provenance” issue: the suggestion assumed $tag_attr is a composite attribute string, but in your codebase it can be "", "defer" or "async", and wp_print_script_tag() handles boolean attributes correctly.
This can be reduced via configuration by tightening the /improve prompt so it:
- verifies where variables come from (diff + surrounding context) before claiming a “critical bug”
- avoids proposing broad parsing logic unless the diff clearly shows
$tag_attris a composite string - respects WP’s boolean-attribute behavior (
async,defer)
Add a repo-level instruction like:
[pr_code_suggestions]
extra_instructions = """\
When reviewing PHP/WordPress code:
- Do NOT assume a variable contains a composite HTML-attribute string unless it is explicitly constructed/validated in the changed code.
- Before flagging a CRITICAL_BUG, confirm the variable’s source (e.g., options/DB values) and allowed values from nearby code/context.
- For wp_print_script_tag()/script attributes, treat 'async' and 'defer' as valid boolean attributes; don't recommend parsing them into key/value pairs unless required by the diff.
- Prefer asking for clarification over asserting breakage when the variable type/value-set is ambiguous.
"""If you want this to apply only to /improve (and not /review), keep it under [pr_code_suggestions] as above. If you can share which tool produced the original comment (likely /improve) and which platform you’re using (GitHub/GitLab/etc.), we can tailor the wording to reduce this specific false-positive pattern even further.
Relevant Sources:
- https://docs.qodo.ai/qodo-documentation/qodo-merge/tools/improve#extra-instructions-and-best-practices
- https://docs.qodo.ai/qodo-documentation/qodo-merge/faq/index#-note-"q:-i-received-an-incorrect-or-irrelevant-suggestion-why"
- https://docs.qodo.ai/qodo-documentation/qodo-merge/usage-guide/additional_configurations#extra-instructions
| $script_attributes = array( | ||
| 'type' => 'text/javascript', | ||
| 'id' => 'Cookiebot', | ||
| 'src' => 'https://consent.cookiebot.com/uc.js', | ||
| 'data-implementation' => 'wp', | ||
| 'data-cbid' => $cbid, | ||
| ); | ||
|
|
||
| if ( $tcf ) { | ||
| $script_attributes['data-framework'] = $tcf; | ||
| } | ||
|
|
||
| if ( $data_regions ) { | ||
| $georegions = array(); | ||
| foreach ( $data_regions as $cbid => $regions ) { | ||
| $georegions[] = '{\'region\':\'' . esc_attr( $regions ) . '\',\'cbid\':\'' . esc_attr( $cbid ) . '\'}'; | ||
| } | ||
| ?> | ||
| <?php if ( (bool) get_option( 'cookiebot-gtm' ) !== false && ! empty( get_option( 'cookiebot-data-layer' ) ) ) : ?> | ||
| data-layer-name="<?php echo esc_attr( get_option( 'cookiebot-data-layer' ) ); ?>" | ||
| <?php endif; ?> | ||
| <?php if ( ! empty( $lang ) ) : ?> | ||
| data-culture="<?php echo esc_attr( strtoupper( $lang ) ); ?>" | ||
| <?php endif; ?> | ||
| <?php if ( $cookie_blocking_mode === 'auto' ) : ?> | ||
| data-blockingmode="auto" | ||
| <?php else : ?> | ||
| <?php echo esc_attr( $tag_attr ); ?> | ||
| <?php endif; ?> | ||
| ></script> | ||
| $script_attributes['data-georegions'] = implode( ',', $georegions ); | ||
| } | ||
|
|
||
| if ( (bool) get_option( 'cookiebot-gtm' ) !== false && ! empty( get_option( 'cookiebot-data-layer' ) ) ) { | ||
| $script_attributes['data-layer-name'] = get_option( 'cookiebot-data-layer' ); | ||
| } | ||
|
|
||
| if ( ! empty( $lang ) ) { | ||
| $script_attributes['data-culture'] = strtoupper( $lang ); | ||
| } | ||
|
|
||
| if ( $cookie_blocking_mode === 'auto' ) { | ||
| $script_attributes['data-blockingmode'] = 'auto'; | ||
| } elseif ( ! empty( $tag_attr ) ) { |
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.
[VALIDATION] User-supplied or option-derived values are assigned to attributes without explicit escaping (e.g. 'data-cbid', 'data-framework', 'data-layer-name', 'data-culture'). Ensure values are sanitized/escaped before insertion (use esc_attr() or cast to string) or validate them. For example: $script_attributes['data-cbid'] = esc_attr( (string) $cbid ); and $script_attributes['data-layer-name'] = esc_attr( (string) get_option('cookiebot-data-layer') ); This prevents injection or malformed attributes when rendering via wp_print_script_tag.
$script_attributes = array(
'type' => 'text/javascript',
'id' => 'Cookiebot',
'src' => 'https://consent.cookiebot.com/uc.js',
'data-implementation' => 'wp',
'data-cbid' => esc_attr( (string) $cbid ),
);
if ( $tcf ) {
$script_attributes['data-framework'] = esc_attr( (string) $tcf );
}
if ( $data_regions ) {
$georegions = array();
foreach ( $data_regions as $region_cbid => $regions ) {
$georegions[] = '{\'region\':\'' . esc_attr( $regions ) . '\',\'cbid\':\'' . esc_attr( $region_cbid ) . '\'}';
}
$script_attributes['data-georegions'] = implode( ',', $georegions );
}
$data_layer = get_option( 'cookiebot-data-layer' );
if ( (bool) get_option( 'cookiebot-gtm' ) !== false && ! empty( $data_layer ) ) {
$script_attributes['data-layer-name'] = esc_attr( (string) $data_layer );
}
if ( ! empty( $lang ) ) {
$script_attributes['data-culture'] = esc_attr( strtoupper( $lang ) );
}| foreach ( $data_regions as $cbid => $regions ) { | ||
| $georegions[] = '{\'region\':\'' . esc_attr( $regions ) . '\',\'cbid\':\'' . esc_attr( $cbid ) . '\'}'; |
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] The foreach uses foreach ( $data_regions as $cbid => $regions ) which re-uses the outer $cbid variable name (the Cookiebot ID). Rename the loop key (e.g. $region_cbid) to avoid variable shadowing and confusion: foreach ( $data_regions as $region_cbid => $regions ) { ... }. This improves readability and avoids accidental bugs if the variable usage order changes.
if ( $data_regions ) {
$georegions = array();
foreach ( $data_regions as $region_cbid => $regions ) {
$georegions[] = '{\'region\':\'' . esc_attr( $regions ) . '\',\'cbid\':\'' . esc_attr( $region_cbid ) . '\'}';
}
$script_attributes['data-georegions'] = implode( ',', $georegions );
}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.
While true this case was present before this patch.
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.
Question:
While true this case was present before this patch.
Answer:
To reduce “this existed before the patch” style refactoring suggestions, configure /improve to focus only on issues introduced by the PR diff and avoid pure readability/naming nits unless they are required for correctness.
Add this to your .pr_agent.toml (or wiki config):
[pr_code_suggestions]
extra_instructions = """\
Only suggest changes that are directly caused by, introduced by, or modified in this PR.
Do not suggest refactors/cleanups for pre-existing code issues unless they create a real bug/security issue in the PR diff.
Prefer correctness/safety issues over readability or naming suggestions.
"""
# Optional: further reduce style-level suggestions
focus_only_on_problems = true
# Optional: filter out low-importance suggestions (tune as needed)
suggestions_score_threshold = 7If you still want occasional refactoring suggestions, keep the threshold lower (e.g., 5–6) and remove/soften the “prefer correctness” line.
Relevant Sources:
- https://docs.qodo.ai/qodo-documentation/qodo-merge/tools/improve#extra-instructions-and-best-practices
- https://docs.qodo.ai/qodo-documentation/qodo-merge/tools/improve#configuration-options
- https://docs.qodo.ai/qodo-documentation/qodo-merge/faq/index#-note-"q:-i-received-an-incorrect-or-irrelevant-suggestion-why"
| if ($consent_attribute) { | ||
| $script_attributes['data-cookieconsent'] = $consent_attribute; | ||
| } |
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.
[VALIDATION] When assigning $consent_attribute into the script attributes use escaping/validation: $script_attributes['data-cookieconsent'] = esc_attr( (string) $consent_attribute ); This prevents malformed attribute values from being output in the inline script tag.
$script_attributes = array(
'type' => $script_type,
);
if ( $consent_attribute ) {
$script_attributes['data-cookieconsent'] = esc_attr( (string) $consent_attribute );
}
ob_start();
?>
<script>
window.<?php echo esc_js( $data_layer ); ?> = window.<?php echo esc_js( $data_layer ); ?> || [];
// ... rest of script body ...
</script>
<?php
wp_print_inline_script_tag( ob_get_clean(), $script_attributes );| ob_start(); | ||
| ?> | ||
| <script type="<?php echo esc_attr( $script_type ); ?>"<?php echo ! $consent_attribute ? '' : ' data-cookieconsent="' . esc_attr( $consent_attribute ) . '"'; ?>> | ||
| window.<?php echo esc_js( $data_layer ); ?> = window.<?php echo esc_js( $data_layer ); ?> || []; | ||
|
|
||
| function gtag() { | ||
| <?php echo esc_js( $data_layer ); ?>.push(arguments); | ||
| } | ||
|
|
||
| gtag("consent", "default", { | ||
| ad_personalization: "denied", | ||
| ad_storage: "denied", | ||
| ad_user_data: "denied", | ||
| analytics_storage: "denied", | ||
| functionality_storage: "denied", | ||
| personalization_storage: "denied", | ||
| security_storage: "granted", | ||
| wait_for_update: 500, | ||
| }); | ||
| gtag("set", "ads_data_redaction", true); | ||
| <?php | ||
| if ( $url_passthrough ) { | ||
| echo /** @lang JavaScript */ | ||
| 'gtag("set", "url_passthrough", true);' . PHP_EOL; | ||
| } | ||
| ?> | ||
| </script> | ||
|
|
||
| <?php wp_print_inline_script_tag( ob_get_clean(), $script_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.
[REFACTORING] You're using ob_start() / ob_get_clean() together with wp_print_inline_script_tag — that's fine, but this relies on WP functions introduced in newer WP versions. Add a fallback for older WP installs (if plugin supports those): if wp_print_inline_script_tag doesn't exist, fallback to echoing a <script> tag with proper escaping. This prevents fatal errors on sites running older WordPress versions.
$script_attributes = array(
'type' => $script_type,
);
if ( $consent_attribute ) {
$script_attributes['data-cookieconsent'] = esc_attr( (string) $consent_attribute );
}
ob_start();
?>
<script>
window.<?php echo esc_js( $data_layer ); ?> = window.<?php echo esc_js( $data_layer ); ?> || [];
// ... rest of script body ...
</script>
<?php
$script = ob_get_clean();
if ( function_exists( 'wp_print_inline_script_tag' ) ) {
wp_print_inline_script_tag( $script, $script_attributes );
} else {
// Fallback for older WP versions.
$type_attr = isset( $script_attributes['type'] ) ? esc_attr( $script_attributes['type'] ) : 'text/javascript';
unset( $script_attributes['type'] );
$attrs = '';
foreach ( $script_attributes as $key => $value ) {
$attrs .= ' ' . esc_attr( $key ) . '="' . esc_attr( $value ) . '"';
}
echo '<script type="' . $type_attr . '"' . $attrs . '>' . $script . '</script>';
}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.
Noting that wp_print_inline_script_tag was added in v5.7.0 and readme.txt does mention the plugin currently supports v4.4
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.
Code Implementation 🛠️
Implementation: Add a WordPress-version fallback for inline script printing by wrapping the existing buffered JS output with a conditional that uses wp_print_inline_script_tag() when available (WP >= 5.7) and otherwise echoes a properly attributed <script> tag. Also ensure the optional data-cookieconsent attribute value is escaped.
| $script_attributes = array( | |
| 'type' => $script_type, | |
| ); | |
| if ( $consent_attribute ) { | |
| $script_attributes['data-cookieconsent'] = esc_attr( (string) $consent_attribute ); | |
| } | |
| ob_start(); | |
| ?> | |
| window.<?php echo esc_js( $data_layer ); ?> = window.<?php echo esc_js( $data_layer ); ?> || []; | |
| function gtag() { | |
| <?php echo esc_js( $data_layer ); ?>.push(arguments); | |
| } | |
| gtag("consent", "default", { | |
| ad_personalization: "denied", | |
| ad_storage: "denied", | |
| ad_user_data: "denied", | |
| analytics_storage: "denied", | |
| functionality_storage: "denied", | |
| personalization_storage: "denied", | |
| security_storage: "granted", | |
| wait_for_update: 500, | |
| }); | |
| gtag("set", "ads_data_redaction", true); | |
| <?php | |
| if ( $url_passthrough ) { | |
| echo /** @lang JavaScript */ | |
| 'gtag("set", "url_passthrough", true);' . PHP_EOL; | |
| } | |
| ?> | |
| <?php | |
| $inline_script = ob_get_clean(); | |
| if ( function_exists( 'wp_print_inline_script_tag' ) ) { | |
| wp_print_inline_script_tag( $inline_script, $script_attributes ); | |
| } else { | |
| // Fallback for older WordPress versions (< 5.7). | |
| $attrs = ''; | |
| foreach ( $script_attributes as $key => $value ) { | |
| if ( null === $value || '' === $value ) { | |
| continue; | |
| } | |
| $attrs .= ' ' . esc_attr( $key ) . '="' . esc_attr( (string) $value ) . '"'; | |
| } | |
| echo '<script' . $attrs . '>' . $inline_script . '</script>'; | |
| } |
See review comment here
|
Reviewed up to commit:29ee06e8fd5bb519f61d7347d0aa5146aab5f895 Additional Suggestionsrc/view/frontend/scripts/cb_frame/cookiebot-js.php, line:31-33get_option('cookiebot-data-layer') is called twice in the condition. Cache the option in a local variable to avoid duplicate option lookups and make the condition clearer, e.g. $data_layer = get_option('cookiebot-data-layer'); if ( (bool) get_option('cookiebot-gtm') !== false && ! empty( $data_layer ) ) { $script_attributes['data-layer-name'] = esc_attr( $data_layer ); }$data_layer = get_option( 'cookiebot-data-layer' );
if ( (bool) get_option( 'cookiebot-gtm' ) !== false && ! empty( $data_layer ) ) {
$script_attributes['data-layer-name'] = esc_attr( (string) $data_layer );
} |
|



User description
User description
Issue #359. This might not be complete but a WIP
PR Type
Enhancement
Description
Replace hardcoded HTML script tags with WordPress core API functions
Use
wp_print_script_tag()for external scripts with attributesUse
wp_print_inline_script_tag()for inline scripts with output bufferingRefactor script attributes into associative arrays for better maintainability
Diagram Walkthrough
File Walkthrough
cookiebot-js.php
Refactor Cookiebot script to use WordPress APIsrc/view/frontend/scripts/cb_frame/cookiebot-js.php
<script>tag towp_print_script_tag()function calllayer-name, culture, blockingmode)
google-consent-mode-js.php
Convert Google Consent Mode script to WordPress APIsrc/view/frontend/scripts/common/google-consent-mode-js.php
<script>tag withwp_print_inline_script_tag()functiondata-cookieconsent
ob_start()andob_get_clean()) to capture inlinescript content
approach
google-tag-manager-js.php
Refactor Google Tag Manager script to WordPress APIsrc/view/frontend/scripts/common/google-tag-manager-js.php
<script>tag withwp_print_inline_script_tag()functiondata-cookieconsent
CodeAnt-AI Description
Replace hardcoded <script> tags with WordPress core script API for analytics and Cookiebot
What Changed
Impact
✅ Fewer malformed script attributes on pages✅ Clearer consent attribute presence for third-party scripts✅ More consistent Cookiebot and GTM script output💡 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.