Skip to content

Conversation

@ronalfy
Copy link

@ronalfy ronalfy commented Dec 13, 2025

Ref #76

This PR resumes the WPCS cleanup:

  1. Adding WPCS Extra/Core rules to phpcs.xml.dist to enforce standards.
  2. Cleaning up index.php to pass standards.
  3. Cleaning up plugin.php to pass standards.
  4. Stricter checks have been placed on conditionals
  5. Several outputs have been escaped to prevent markup hijacking.
  6. Snakecase has been converted to underscores per WPCS.

I've tested:

  1. With the accordion/without.
  2. With markup wrapper enabled/disabled.
  3. Custom titles in the TOC in the block options.

I could use some more help testing the DOM XML Parsing features, as I removed an error suppressor and did a try/catch instead.

Thank you for reviewing!

@mtoensing
Copy link
Owner

mtoensing commented Dec 14, 2025

I've created a stress test and it seems to work fine. If you have an answer to my question abvove I thing we can merge it. Here is my test case

<!-- wp:paragraph -->
<p>This is a stress test for complex headline generation.</p>
<!-- /wp:paragraph -->

<!-- wp:simpletoc/toc /-->

<!-- wp:heading {"level":3} -->
<h3 class="wp-block-heading"><br />Über-komplexe Überschrift №1: Größen, Maße &amp; „Sonderzeichen“ (50 %–75 %)<br /></h3>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>Yes, we use chaotic nesting.</p>
<!-- /wp:paragraph -->

<!-- wp:heading {"level":1} -->
<h1 class="wp-block-heading"><br /><br />¿Qué pasó con “El Niño”? – Año 2024/2025 🇪🇸🌊</h1>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p><br />We use empty spaces in headline</p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2 class="wp-block-heading"><br />中文标题测试:汉字、全角符号(%&@)与空格</h2>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>Some japanese<br /><br />العنوان الرئيسي — اختبار الاتجاه من اليمين إلى اليسار (RTL)</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Some arabic from right to left.</p>
<!-- /wp:paragraph -->

<!-- wp:heading {"level":6} -->
<h6 class="wp-block-heading"><br /><strong>E</strong>moji 🤯🚀 headline with ZERO-WIDTH chars​ and non-breaking spaces here<br /><br /></h6>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p><br /></p>
<!-- /wp:paragraph -->

@ronalfy
Copy link
Author

ronalfy commented Dec 15, 2025

@mtoensing I'm not seeing the question to respond to. Can you please point me in the right direction?

Copy link
Owner

@mtoensing mtoensing left a comment

Choose a reason for hiding this comment

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

@ronalfy Yes, this is an important change. I used the @ syntax because I had problem with try catch in an old php version. Since I see that you generally know what you are doing: is this worth the risk? Or do you say "Relax, there is no risk. This works and it is best practice?

Maybe this was not visible before I set "Request changes" but.I am not sure if we need changes.

* @return string The modified HTML string with the closing tag(s) added
*/
function add_accordion_end( $html, $attributes ) {
// Check if accordion is enabled either through the function arguments or the options
Copy link
Owner

Choose a reason for hiding this comment

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

@ronalfy Yes, this is an important change. I used the @ syntax because I had problem with try catch in an old php version. Since I see that you generally know what you are doing: is this worth the risk? Or do you say "Relax, there is no risk. This works and it is best practice?

Copy link
Author

Choose a reason for hiding this comment

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

@mtoensing I can double-check this. Do you recall which version of PHP? Does this plugin have a minimum supported PHP?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes minimum is 7.3. see readme

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I've double-checked the readme versions. I tested as far back as WordPress 5.9, as that's the minimum supported version according to the readme.

Here's what I've found.

I tested SimpleTOC 6.9.1 (prior to these changes) and the latest 6.9.4.

From my tests, the minimum working version of the plugin is WordPress 6.6.
The minimum supported PHP version of the plugin is: 7.2.

If these requirements can be updated, then the suppressed error message flag can safely be removed as I'm not receiving any errors with the above setup.

Copy link
Author

Choose a reason for hiding this comment

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

Here are the WP supported versions if that helps. WP 6.6 supports a minimum of 7.2, which makes sense.

https://make.wordpress.org/core/handbook/references/php-compatibility-and-wordpress-versions/

Copy link
Author

@ronalfy ronalfy Dec 15, 2025

Choose a reason for hiding this comment

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

I'll add that versions before WP 6.6 don't even show SimpleTOC in the block editor with no obvious error messages.

image

@mtoensing mtoensing self-assigned this Dec 15, 2025
@mtoensing mtoensing merged commit acab68e into mtoensing:master Dec 15, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants