Automattic / syntaxhighlighter

WordPress plugin that makes it easy to post syntax-highlighted code snippets.
https://alex.blog/wordpress-plugins/syntaxhighlighter/
239 stars 78 forks source link

Fix nested shortcode bugs #261

Closed gogdzl closed 1 year ago

gogdzl commented 1 year ago

Fixes #32, #26 and #71 - Fix nested shortcode bugs

This PR addresses issue #32 (Nested shortcode problem). It ensures the plugin doesn't run its own shortcodes multiple times and corrects the original escape method used to replace open square bracket symbols. The original method replaced the [ symbols with [ to prevent nested shortcodes from running. However, as indicated in the docs (https://developer.wordpress.org/reference/functions/do_shortcode/ and https://developer.wordpress.org/reference/functions/unescape_invalid_shortcodes/), [ is converted back to [. To address this, we've changed [ to [. This modification resolves the issue while preserving the initial escape intention.

Additionally, this PR solves issues #26 and #71 by relying on WordPress's shortcode parsing. This lets SyntaxHighlighter's shortcode strings (e.g., [c]) be used within other shortcodes without interference from SyntaxHighlighter. This solution aims to prevent compatibility issues as illustrated in #71.

The solution proposed for #26 and #71 is somewhat intrusive, as it rewrites the shortcode strings for all shortcodes other than SyntaxHighlighter's. I've considered various scenarios and believe the logic to be robust against edge cases:

If the fix to #26 and #71 is deemed to be too intrusive, the first two commits fix #32. Nonetheless, I believe the code here is as safe as the rest of SyntaxHighlighter's code when it comes to compatibility.

Changes proposed in this Pull Request

Testing instructions

To test instructions from #260 can be used.

As an aditional test this shortcode structure can be used:

function attribute_testing_shortcode( $atts ) {
    $a = shortcode_atts( array(
        'att1' => '',
        'att2' => '',
        'att3' => '',
    ), $atts );

    $response = '';
    if ( ! empty( $a['att1'] ) ) {
        $response .= ' ' . $a['att1'];
    }
    if ( ! empty( $a['att2'] ) ) {
        $response .= ' ' . $a['att2'];
    }
    if ( ! empty( $a['att3'] ) ) {
        $response .= ' ' . $a['att3'];
    }
    return $response;
}
add_shortcode( 'attribute_testing', 'attribute_testing_shortcode' );
<!-- wp:shortcode -->
[c] int i = data[c]; [/c]
<!-- /wp:shortcode -->

<!-- wp:shortcode -->
[attribute_testing att1=one att2='two"' att3="three'''" ]
<!-- /wp:shortcode -->

<!-- wp:shortcode -->
[yell] Yelling data[c] [/yell]
<!-- /wp:shortcode -->

<!-- wp:shortcode -->
[yell] [[c]] [/yell]
<!-- /wp:shortcode -->
gogdzl commented 1 year ago

Any ideas on how to solve this? Maybe should we treat this as a different issue? Not sure.

This is an entirely different bug. It's one of the issues I mentioned in the comment I left in #260:

Additionally, I stumbled upon some unreported bugs when mixing blocks and shortcodes. I'll be opening detailed issues about those later.

But I haven't got the chance to create detailed issues to report them.

fjorgemota commented 1 year ago

Sounds good, thanks!

@aaronfc @jom Could any of you please test if this PR fixes the cases you tested on #260, please? I tested it here and it seems to be working well, but I'd love to have a second look at this. Thanks!

fjorgemota commented 1 year ago

Given that @gogdzl is on WCUS and that this PR is approved, I'll merge this. Thanks for the PR, again, @gogdzl! And thanks @jom and @aaronfc for the extra review!