Add a maximum nesting depth for collapsible details

AI-generated summary

scavin proposes adding a configurable maximum nesting depth for the built-in [details] feature, citing a real-world case where 24 levels of nesting caused severe browser performance issues, Markdown endpoint timeouts (up to 31 seconds), and server errors. To address this, scavin created a temporary plugin limiting nesting to two levels and suggests a details_max_nesting_depth site setting for administrators.

Ethsim2 identifies the root cause as exponential processing in MarkdownEndpoint::CookedProcessor#replace_details and submits a draft PR #44461 that reduces recursive conversions, resulting in a 31× speedup. Additionally, Ethsim2 is implementing the requested details_max_nesting_depth setting, defaulting to 0 (unlimited) for backward compatibility. scavin recommends separating the performance fix and the new setting into two distinct PRs to allow the critical performance patch to be merged independently. Ethsim2 agrees to follow the team’s preference regarding the PR structure.

I’d like to suggest adding a maximum nesting depth for the built-in [details] feature, or introducing a site setting that allows administrators to configure this limit.

The problem

Recently, a user on my Discourse forum created a post containing 24 levels of nested [details] sections.

This caused several problems:

  1. Browser performance: Users reported that expanding multiple levels caused Firefox to become extremely slow or unresponsive.
  2. Markdown endpoint timeouts: Accessing the topic’s .md endpoint became extremely slow. We observed requests taking 19.8 seconds and 31.1 seconds.
  3. Server errors: The Markdown endpoint sometimes returned an “Oops” error page. The Discourse logs also showed Pitchfork worker timeout warnings during Markdown conversion.
  4. Potential resource exhaustion: Since search engines and AI crawlers frequently request .md endpoints, a deeply nested post could repeatedly trigger expensive processing and consume server resources.

After removing the problematic post, the Markdown endpoint returned HTTP 200 and responded in approximately 1.7 seconds.

The affected topic was:

https://meta.appinn.net/t/topic/87672

(The problematic reply has already been removed.)

Suggested improvement

I think it would be useful to either:

  • Limit [details] nesting to a reasonable depth, such as 2 or 3 levels.
  • Add a site setting, such as details_max_nesting_depth, allowing administrators to configure the maximum nesting depth.

For backward compatibility, the setting could default to unlimited, while allowing administrators to enforce a limit when needed.

If users exceed the configured limit, Discourse could reject the post with a clear validation message.

Temporary workaround: a plugin

I’ve created a small plugin that limits [details] nesting to a maximum of 2 levels:

GitHub - scavin/discourse-details-depth-limit · GitHub

The plugin validates posts on the server side and rejects posts containing more than two levels of nested collapsible sections.

I’ve tested it successfully in my local Discourse development environment.

For administrators experiencing similar issues, this plugin may serve as a temporary workaround until an official solution becomes available.

I believe a configurable limit in the built-in Details plugin would be a useful safeguard against accidental or excessive nesting.

2 Likes

Hi @scavin, thanks for reporting this and for providing the reproduction details.

I’ve been investigating the Markdown endpoint slowdown you described and identified an issue in MarkdownEndpoint::CookedProcessor#replace_details: nested <details> elements were being converted repeatedly, resulting in exponential processing.

I’ve opened a draft upstream PR with a fix:

At eight nesting levels, the fix reduced recursive conversions from 255 to 8, with approximately a 31× speedup in my local benchmark. The PR has passed its initial GitHub CI checks.

I’m also working on your proposed details_max_nesting_depth site setting within the built-in Details plugin, using 0 for unlimited nesting to preserve existing behaviour. The initial implementation is written, but its integration tests are still in progress.

The performance fix addresses the server-side Markdown conversion problem; the configurable limit would provide an additional safeguard, particularly for browser-side performance.

I’d welcome feedback on the proposed setting and whether it would be preferable to include it in the same PR or submit it separately.

1 Like

Thanks for the quick investigation and fix!
I think separating them into two PRs would be better, so the performance fix can be reviewed and merged independently without waiting for the new setting.
I also support the configurable depth limit, with 0 as the default for backward compatibility.

Thanks @scavin. I can see the benefit of separating them, although both changes address your same underlying report and are now implemented and CI-tested. I have maintainer edits enabled on all my PRs, so I’m happy to follow the Discourse team’s preference, whether that’s reviewing both commits together or separating them.

1 Like