Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix nested shortcode bugs #261

Merged
merged 6 commits into from
Aug 25, 2023
Merged

Fix nested shortcode bugs #261

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
34dfa75
Change escape code to preserve the escaped string after do_shortcode …
gogdzl Aug 14, 2023
455514e
Remove our shortcodes from the original ones to avoid running them mu…
gogdzl Aug 14, 2023
fedfd0c
Add another hack to other shortcodes to return the entire shortcode s…
gogdzl Aug 14, 2023
736f10a
Remove unneeded parameter
gogdzl Aug 21, 2023
cefde48
Change method employed to return the entire shortcode unchanged for a…
gogdzl Aug 22, 2023
392bfae
Stop adding the same filter multiple times
gogdzl Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 52 additions & 9 deletions syntaxhighlighter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,11 @@ function output_shortcodes_for_tinymce() {
*
* First we need to clear out all existing shortcodes, then register
* just this plugin's ones, process them, and then restore the original
* list of shortcodes.
* list of shortcodes. Additionally, we have to add another hack to other
* shortcodes, such as [gallery], to return the entire shortcode string as
* it appears in the original content to allow SyntaxHighlighter's shortcode
* strings (e.g., [c]) be used within other shortcodes without interference
* from SyntaxHighlighter.
*
* To make matters more complicated, if someone has done [[code]foo[/code]]
* in order to display the shortcode (not render it), then do_shortcode()
Expand All @@ -639,10 +643,6 @@ function output_shortcodes_for_tinymce() {
* even more brackets escaped shortcodes in order to result in
* the shortcodes actually being displayed instead rendered.
*
* We only need to do this for this plugin's shortcodes however
* as all other shortcodes such as [[gallery]] will be untouched
* by this pass of do_shortcode.
*
* Phew!
*
* @param string $content The post content.
Expand All @@ -659,16 +659,21 @@ function shortcode_hack( $content, $callback, $ignore_html = true ) {
return $content;
}

// Backup current registered shortcodes and clear them all out
$orig_shortcode_tags = $shortcode_tags;
// Backup current registered shortcodes and clear them all out (we do not backup our own, because we will add and parse them below)
$orig_shortcode_tags = array_diff_key( $shortcode_tags, array_flip( $this->shortcodes ) );
remove_all_shortcodes();

// Register all of this plugin's shortcodes
foreach ( $this->shortcodes as $shortcode ) {
add_shortcode( $shortcode, $callback );
}

$regex = '/' . get_shortcode_regex( $this->shortcodes ) . '/';
// Register all other shortcodes, ensuring their content remains unchanged using yet another hack.
foreach ( $orig_shortcode_tags as $shortcode_tagname => $shortcode ) {
add_shortcode( $shortcode_tagname, array( $this, 'return_entire_shortcode_callback' ) );
}

$regex = '/' . get_shortcode_regex( array_merge( $this->shortcodes, array_keys( $orig_shortcode_tags ) ) ) . '/';
gogdzl marked this conversation as resolved.
Show resolved Hide resolved

// Parse the shortcodes (only this plugins's are registered)
if ( $ignore_html ) {
Expand Down Expand Up @@ -749,6 +754,44 @@ function shortcode_hack_extra_escape_escaped_shortcodes_and_parse( $match ) {
return do_shortcode_tag( $match );
}

/**
* Callback function to return the entire shortcode string as it appears in content.
*
* @param array $atts Array of attributes passed to the shortcode.
* @param string|null $content Content enclosed between the opening and closing shortcode tags. Default is null.
* @param string $tag Shortcode name/tag.
*
* @return string The complete shortcode string including the opening and closing tags, attributes, and content.
*/
function return_entire_shortcode_callback( $atts, $content = null, $tag = '' ) {
$shortcode_string = '[' . $tag;

if ( ! empty( $atts ) ) {
foreach ( $atts as $key => $value ) {
if ( strpos( $value, "'" ) !== false && strpos( $value, '"' ) === false ) {
// Use double quotes if value contains a single quote but not a double quote.
$shortcode_string .= ' ' . $key . '="' . $value . '"';
} elseif ( strpos( $value, '"' ) !== false && strpos( $value, "'" ) === false ) {
// Use single quotes if value contains a double quote but not a single quote.
$shortcode_string .= ' ' . $key . "='" . $value . "'";
} elseif ( strpos( $value, "'" ) !== false && strpos( $value, '"' ) !== false ) {
// If value contains both types of quotes, use double quotes and escape inner double quotes.
$pattern = '/(?<!\\\\)"/'; // This pattern looks for double quotes that aren't preceded by a backslash.
$escaped_value = preg_replace( $pattern, '\"', $content );
$shortcode_string .= ' ' . $key . '="' . $escaped_value . '"';
} else {
$shortcode_string .= ' ' . $key . '="' . $value . '"';
}
}
}

$shortcode_string .= ']';

if ( $content !== null ) {
$shortcode_string .= $content . '[/' . $tag . ']';
}
return $shortcode_string;
}
Copy link
Member

Choose a reason for hiding this comment

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

As someone that almost came close to this while working on #260, I must ask:

WDYT if, instead of trying to re-create the shortcode content in a shortcode callback, we force the escape on the shortcodes outside $this->shortcodes in shortcode_hack_extra_escape_escaped_shortcodes?

I'm just a bit worried that this might cause weird conflicts with other plugins, especially as the fact that shortcodes rely on regex..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

WDYT if, instead of trying to re-create the shortcode content in a shortcode callback, we force the escape on the shortcodes outside $this->shortcodes in shortcode_hack_extra_escape_escaped_shortcodes?

I considered an approach similar to what you described. However, it introduced even more edge cases to handle. That's why I opted to rewrite everything, letting WordPress manage the registered shortcodes/nested shortcodes.

I'm just a bit worried that this might cause weird conflicts with other plugins, especially as the fact that shortcodes rely on regex..

As I highlighted in the PR description, I recognize that this is an intrusive approach. I took extra care during the implementation to address the edge cases specified in the description, and based on my findings, it seems like it won't break anything.

I share your concerns about potential conflicts. If this is deemed too intrusive, one alternative could be introducing a new setting to exclude all shortcodes except for [code]. The idea of removing [c] was even discussed in #72. To address @Viper007Bond's concerns about breaking sites (#71), we could activate this shortcode removal only when the setting is enabled, keeping it off by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we discussed, a less intrusive method was employed in cefde48. Thanks for pointing out the existence of pre_do_shortcode_tag!

Copy link
Member

Choose a reason for hiding this comment

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

Great, thanks!


// The main filter for the post contents. The regular shortcode filter can't be used as it's post-wpautop().
function parse_shortcodes( $content ) {
Expand Down Expand Up @@ -1377,7 +1420,7 @@ function shortcode_callback( $atts, $code = '', $tag = false ) {
$code = ( false === strpos( $code, '<' ) && false === strpos( $code, '>' ) && 2 == $this->get_code_format( $post ) ) ? strip_tags( $code ) : htmlspecialchars( $code );

// Escape shortcodes
$code = preg_replace( '/\[/', '&#91;', $code );
$code = preg_replace( '/\[/', '&#x5B;', $code );
gogdzl marked this conversation as resolved.
Show resolved Hide resolved

$params[] = 'notranslate'; // For Google, see http://otto42.com/9k

Expand Down