Skip to content

Conversation

@bwmatter
Copy link

Added in extra validation in the simpletoc_sanitize_string() function to ensure the generated ID attribute validates (i.e. begins with a letter.)

Added in extra validation in the simpletoc_sanitize_string() function to ensure the generated ID attribute validates (i.e. begins with a letter.)
@mtoensing
Copy link
Owner

The intention is good (ensure IDs don’t start with a digit), but the current patch has a few bugs and will over-prefix/over-encode. I’d request changes before merging.

Issues in the PR

Regex is wrong for “begins with a letter”. "/^[_a-zA-Z]+$/" matches the entire string and forbids digits/dashes entirely. You only want to check the first character.

Double encoding. $urlencoded = urlencode($sanitized_string); is executed in both branches and again after the if → redundant / risk of future double-encode changes.

Encoding policy. Encoding here is questionable: sanitize_title_with_dashes() already yields URL/ID-safe slugs. Returning an encoded value can lead to later double encoding when appended to URLs.

Empty slug case. If a heading becomes empty after sanitization (e.g., only emojis), there’s no fallback.

@mtoensing mtoensing closed this Dec 4, 2025
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.

2 participants